diff --git a/src/cli/index.ts b/src/cli/index.ts index 93c963833b..9d37cb5c21 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -732,6 +732,9 @@ function reportRestartFailure(result: Extract if (code === "restart_capability_unsupported") { console.error("❌ The running proxy predates process-bound restart support; no unsafe fallback was attempted."); console.error(" After confirming this home owns the proxy, run `ocx stop` and then `ocx start` once."); + } else if (code === "restart_version_skew") { + console.error("❌ The running proxy reports a different OpenCodex version than this CLI; restarting in place would respawn the old installation."); + console.error(" Run `ocx stop` and then `ocx start` from this installation instead."); } else { console.error("❌ Proxy restart request could not be confirmed; no fallback stop/start was attempted."); } diff --git a/src/cli/system-restart-client.ts b/src/cli/system-restart-client.ts index fbc7c4548f..5f0e1ad2ab 100644 --- a/src/cli/system-restart-client.ts +++ b/src/cli/system-restart-client.ts @@ -22,6 +22,8 @@ import { type LiveProxy, } from "../server/proxy-liveness"; import type { ProxyRestartRequestOutcome } from "./tray-proxy"; +import { packageVersion } from "./help"; +import { computeVersionSkew } from "./version-skew"; export const SYSTEM_RESTART_REQUEST_TIMEOUT_MS = 5_000; export const SYSTEM_RESTART_ATTESTATION_TIMEOUT_MS = 4_000; @@ -32,6 +34,8 @@ export interface BoundSystemRestartDeps { findLive?: typeof findLiveProxy; createChallenge?: () => string; now?: () => number; + /** Invoking CLI version for the skew guard; defaults to this bundle's package version. */ + cliVersion?: string; } function rejected(code: string): ProxyRestartRequestOutcome { @@ -107,6 +111,18 @@ export async function requestBoundSystemRestart( return rejected("restart_capability_unsupported"); } + // An in-place restart respawns the live process from its own installation + // (selfLaunchArgv in server/management/system-restart.ts), so a restart accepted + // from a different-version CLI would keep the OLD build serving while reporting + // success (#4522). Both sides already publish exactly the data doctor's skew + // diagnosis compares (packageVersion vs the /healthz version), so reuse that + // comparison and refuse before POST. Placeholder versions (unknown/0.0.0) are + // "cannot compare", not mismatch, and keep the existing behavior. + const proxyVersion = typeof body.version === "string" ? body.version : undefined; + if (computeVersionSkew(deps.cliVersion ?? packageVersion(), proxyVersion).skewed) { + return rejected("restart_version_skew"); + } + let observed: LiveProxy | null; try { observed = await (deps.findLive ?? findLiveProxy)({ deadlineAt, nowFn: now }); diff --git a/tests/cli/system-restart-client.test.ts b/tests/cli/system-restart-client.test.ts index f7c8c7657a..07b9e3a4e5 100644 --- a/tests/cli/system-restart-client.test.ts +++ b/tests/cli/system-restart-client.test.ts @@ -52,6 +52,9 @@ function successfulDeps() { findLive: async () => target, createChallenge: () => challenge, now: () => 1_000, + // Matches the /healthz fixture version below so the skew guard stays out of the way; + // the dedicated skew tests override it explicitly. + cliVersion: "test", }, }; } @@ -144,6 +147,81 @@ describe("bound system restart client", () => { expect(setup.requests).toHaveLength(1); }); + test("refuses a restart through a CLI whose version differs from the attested proxy", async () => { + for (const [proxyVersion, cliVersion] of [ + ["2.49.0", "2.53.0"], + ["2.53.0", "2.49.0"], + ["test", "2.53.0"], + ] as const) { + const setup = successfulDeps(); + setup.deps.cliVersion = cliVersion; + setup.deps.fetchImpl = (async (input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + setup.requests.push({ url, init }); + if (url.endsWith("/healthz")) { + const response = successfulDepsResponse(setup.secret, setup.challenge); + const body = await response.json() as Record; + body.version = proxyVersion; + return new Response(JSON.stringify(body), { + status: 200, + headers: response.headers, + }); + } + throw new Error("POST must not be attempted"); + }) as typeof fetch; + + const outcome = await requestBoundSystemRestart(target, 10_000, setup.deps); + expect(outcome).toMatchObject({ accepted: false, uncertain: false }); + expect(outcome.accepted ? "" : (outcome.error as Error).message) + .toBe("restart_version_skew"); + expect(setup.requests).toHaveLength(1); + } + }); + + test("allows a restart when the invoking CLI matches the attested proxy version", async () => { + const setup = successfulDeps(); + setup.deps.cliVersion = "2.53.0"; + setup.deps.fetchImpl = (async (input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + setup.requests.push({ url, init }); + if (url.endsWith("/healthz")) { + const response = successfulDepsResponse(setup.secret, setup.challenge); + const body = await response.json() as Record; + body.version = "2.53.0"; + return new Response(JSON.stringify(body), { + status: 200, + headers: response.headers, + }); + } + return new Response(JSON.stringify({ success: true }), { status: 202 }); + }) as typeof fetch; + + expect(await requestBoundSystemRestart(target, 10_000, setup.deps)).toEqual({ accepted: true }); + expect(setup.requests).toHaveLength(2); + }); + + test("treats a placeholder proxy version as incomparable and keeps the restart path", async () => { + const setup = successfulDeps(); + setup.deps.cliVersion = "2.53.0"; + setup.deps.fetchImpl = (async (input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + setup.requests.push({ url, init }); + if (url.endsWith("/healthz")) { + const response = successfulDepsResponse(setup.secret, setup.challenge); + const body = await response.json() as Record; + body.version = "0.0.0"; + return new Response(JSON.stringify(body), { + status: 200, + headers: response.headers, + }); + } + return new Response(JSON.stringify({ success: true }), { status: 202 }); + }) as typeof fetch; + + expect(await requestBoundSystemRestart(target, 10_000, setup.deps)).toEqual({ accepted: true }); + expect(setup.requests).toHaveLength(2); + }); + test("refuses to POST when the live target changes after attestation", async () => { const setup = successfulDeps(); setup.deps.findLive = async () => ({ ...target, pid: 4343 });