Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions apps/server/src/assisted-update-store.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,10 +2,7 @@ import { randomBytes } from "node:crypto";
import os from "node:os";
import path from "node:path";
import { mkdir, readFile, rename, writeFile } from "node:fs/promises";
import type {
AssistedUpdateMetadata,
RequiredCheckName,
} from "./release-metadata.js";
import type { AssistedUpdateMetadata } from "./release-metadata.js";
import type { CheckResult } from "./release-checks.js";
import type { UpdateMigrationManifest } from "./update-migrations.js";

Expand DownExpand Up@@ -54,7 +51,9 @@ export type AssistedUpdateState = {
* `metadata` for the legacy single-block flow.
*/
migrations: UpdateMigrationManifest[] | null;
requiredChecks: RequiredCheckName[];
/** Usually RequiredCheckName values; manifests from a newer release may
* name checks this runtime doesn't know (they fail closed when run). */
requiredChecks: string[];
phase: AssistedPhase;
/** Random nonce; the launched agent uses this to authenticate phase POSTs. */
token: string;
Expand Down
28 changes: 20 additions & 8 deletions apps/server/src/assisted-update.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,7 +16,6 @@ import {
type AssistedUpdateState,
} from "./assisted-update-store.js";
import { runRequiredChecks, type CheckContext } from "./release-checks.js";
import type { RequiredCheckName } from "./release-metadata.js";
import type { UpdateMigrationManifest } from "./update-migrations.js";
import { markMigrationsApplied } from "./applied-migrations-store.js";
import { clearEvaluatorCache } from "./update-migrations-evaluator.js";
Expand DownExpand Up@@ -74,9 +73,17 @@ export async function buildAssistedUpdateContext(
"buildAssistedUpdateContext requires either migrations or metadata"
);
}
const requiredChecks: RequiredCheckName[] = migrations
const requiredChecks: string[] = migrations
? unionMigrationChecks(migrations)
: normalizeRequiredChecks(metadata!);
// Always prove the running executable's version, even when no manifest
// asks for it. Manifests can't name this check yet (pre-v0.33 runtimes
// reject unknown check names at parse time), and release.json-based
// checks alone false-green when a pinned service entrypoint restarts back
// into the old binary.
if (!requiredChecks.includes("running_version")) {
requiredChecks.push("running_version");
}

const state: AssistedUpdateState = {
tag: input.tag,
Expand DownExpand Up@@ -111,11 +118,9 @@ export async function buildAssistedUpdateContext(
return { state, prompt };
}

function unionMigrationChecks(
migrations: UpdateMigrationManifest[]
): RequiredCheckName[] {
const seen = new Set<RequiredCheckName>();
const ordered: RequiredCheckName[] = [];
function unionMigrationChecks(migrations: UpdateMigrationManifest[]): string[] {
const seen = new Set<string>();
const ordered: string[] = [];
for (const m of migrations) {
for (const c of m.validation.requiredChecks) {
if (seen.has(c)) continue;
Expand DownExpand Up@@ -201,7 +206,14 @@ export async function runAndRecordChecks(
state: AssistedUpdateState,
ctx: CheckContext
): Promise<AssistedUpdateState> {
const results = await runRequiredChecks(state.requiredChecks, ctx);
// Enforce running_version at check-run time as well as launch time: a run
// launched by an older server (whose state.requiredChecks predates the
// check) is often validated by the freshly restarted target binary — this
// code — so appending here closes the gap for in-flight upgrades.
const names = state.requiredChecks.includes("running_version")
? state.requiredChecks
: [...state.requiredChecks, "running_version"];
const results = await runRequiredChecks(names, ctx);
state.checks = results;
state.updatedAt = new Date().toISOString();
await writeAssistedUpdateState(state);
Expand Down
102 changes: 90 additions & 12 deletions apps/server/src/release-checks.ts
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
import { readFile, stat } from "node:fs/promises";
import { lstat, readFile } from "node:fs/promises";
import https from "node:https";
import os from "node:os";
import path from "node:path";
Expand All@@ -14,13 +14,15 @@ export type CheckContext = {
};

export type CheckResult = {
name: RequiredCheckName;
/** Usually a RequiredCheckName; a manifest authored for a newer runtime
* may name a check this build doesn't know (those fail closed). */
name: string;
ok: boolean;
message: string;
};

export async function runRequiredChecks(
names: RequiredCheckName[],
names: ReadonlyArray<string>,
ctx: CheckContext
): Promise<CheckResult[]> {
const results: CheckResult[] = [];
Expand All@@ -38,11 +40,8 @@ export async function runRequiredChecks(
return results;
}

async function runCheck(
name: RequiredCheckName,
ctx: CheckContext
): Promise<CheckResult> {
switch (name) {
async function runCheck(name: string, ctx: CheckContext): Promise<CheckResult> {
switch (name as RequiredCheckName) {
case "expected_runtime_artifact":
return checkRuntimeArtifact(ctx);
case "service_entrypoint":
Expand All@@ -53,13 +52,34 @@ async function runCheck(
return checkHealthEndpoint(ctx);
case "version_converged":
return checkVersionConverged(ctx);
case "running_version":
return checkRunningVersion(ctx);
default:
// Fail closed: a manifest authored for a newer runtime may require a
// check this build cannot evaluate. Passing it silently would defeat
// the point of a required check.
return {
name,
ok: false,
message: `unknown required check "${name}" — this runtime cannot evaluate it`,
};
}
}

async function checkRuntimeArtifact(ctx: CheckContext): Promise<CheckResult> {
const runtime = fixedRuntimePath(ctx.serverDir);
try {
const info = await stat(runtime);
// lstat, not stat: a symlink at the fixed path (e.g. a legacy pin to a
// versioned dist/bun binary) must not pass as the activated runtime —
// atomic replacement and .previous rollback assume a regular file.
const info = await lstat(runtime);
if (info.isSymbolicLink()) {
return {
name: "expected_runtime_artifact",
ok: false,
message: `${runtime} is a symlink; the fixed runtime must be a regular executable`,
};
}
return info.isFile()
? {
name: "expected_runtime_artifact",
Expand DownExpand Up@@ -225,9 +245,13 @@ function isLoopbackHttps(url: string): boolean {

async function fetchViaGlobal(
url: string
): Promise<{ status: number; body: string }> {
): Promise<{ status: number; body: string; versionHeader: string | null }> {
const res = await fetch(url, { signal: AbortSignal.timeout(5_000) });
return { status: res.status, body: await res.text() };
return {
status: res.status,
body: await res.text(),
versionHeader: res.headers.get("x-dispatch-version"),
};
}

// The same-process self-check legitimately hits the local server's
Expand All@@ -238,7 +262,7 @@ async function fetchViaGlobal(
// future config points it at a remote host.
function fetchLoopbackHttps(
url: string
): Promise<{ status: number; body: string }> {
): Promise<{ status: number; body: string; versionHeader: string | null }> {
return new Promise((resolve, reject) => {
const parsed = new URL(url);
const req = https.request(
Expand All@@ -254,9 +278,13 @@ function fetchLoopbackHttps(
const chunks: Buffer[] = [];
res.on("data", (c: Buffer) => chunks.push(c));
res.on("end", () => {
const header = res.headers["x-dispatch-version"];
resolve({
status: res.statusCode ?? 0,
body: Buffer.concat(chunks).toString("utf8"),
versionHeader: Array.isArray(header)
? (header[0] ?? null)
: (header ?? null),
});
});
res.on("error", reject);
Expand All@@ -270,6 +298,56 @@ function fetchLoopbackHttps(
});
}

/**
* Prove the executable that is actually serving requests is the target
* version. Every API response carries the build-time package version in the
* X-Dispatch-Version header, so this cannot be spoofed by an on-disk record
* — release.json only says what was *deployed*, not what is *running* (a
* version-pinned service entrypoint can restart straight back into the old
* binary while release.json claims the target).
*/
async function checkRunningVersion(ctx: CheckContext): Promise<CheckResult> {
const url = ctx.healthUrl ?? "http://127.0.0.1:6767/api/v1/health";
const expected = ctx.targetTag.replace(/^v/, "");
try {
const { status, versionHeader } = isLoopbackHttps(url)
? await fetchLoopbackHttps(url)
: await fetchViaGlobal(url);
if (status < 200 || status >= 300) {
return {
name: "running_version",
ok: false,
message: `${url} returned ${status}`,
};
}
const running = versionHeader?.trim();
if (!running) {
return {
name: "running_version",
ok: false,
message: `no X-Dispatch-Version header from ${url} — cannot prove the running executable's version (runtime predates version reporting or is not the target binary)`,
};
}
return running === expected
? {
name: "running_version",
ok: true,
message: `running executable reports ${running} (target ${ctx.targetTag})`,
}
: {
name: "running_version",
ok: false,
message: `running executable reports ${running}, expected ${expected} (target ${ctx.targetTag})`,
};
} catch (err) {
return {
name: "running_version",
ok: false,
message: err instanceof Error ? err.message : String(err),
};
}
}

async function checkVersionConverged(ctx: CheckContext): Promise<CheckResult> {
const record = await readReleaseStore().catch(() => null);
if (!record) {
Expand Down
7 changes: 7 additions & 0 deletions apps/server/src/release-metadata.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,13 @@ export const REQUIRED_CHECK_NAMES = [
"service_restarted",
"health_endpoint",
"version_converged",
// Proves the actual running executable's baked-in version (via the
// X-Dispatch-Version response header), not what release.json claims.
// Runtimes before v0.33 reject unknown check names when parsing update
// migration manifests, so keep this OUT of update-migrations/*.yaml
// requiredChecks until the pre-v0.33 population is gone — the assisted
// framework enforces it implicitly (see runAndRecordChecks).
"running_version",
] as const;
export type RequiredCheckName = (typeof REQUIRED_CHECK_NAMES)[number];

Expand Down
12 changes: 10 additions & 2 deletions apps/server/src/update-migrations.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,7 +2,6 @@ import { readdir, readFile } from "node:fs/promises";
import path from "node:path";
import { parse as parseYaml } from "yaml";
import { z } from "zod";
import { REQUIRED_CHECK_NAMES } from "./release-metadata.js";

/**
* Persistent install-update migration manifest. Migrations live in
Expand All@@ -14,7 +13,16 @@ import { REQUIRED_CHECK_NAMES } from "./release-metadata.js";

const MANIFEST_FILE_RE = /^(\d+)-([a-z0-9][a-z0-9-]*)\.ya?ml$/i;

const RequiredCheckSchema = z.enum(REQUIRED_CHECK_NAMES);
// Deliberately NOT an enum of REQUIRED_CHECK_NAMES. Manifests ship inside
// the *target* release tarball but are parsed by the *currently installed*
// runtime — a strict enum here would make every older install reject a
// manifest the moment a new check name is introduced, silently dropping the
// whole migration from its pending set. Accept any well-formed name at
// parse time; `runRequiredChecks` fails closed on names this runtime
// doesn't implement.
const RequiredCheckSchema = z
.string()
.regex(/^[a-z0-9][a-z0-9_]*$/, "check name must be lowercase snake_case");

export const UpdateMigrationManifestSchema = z.object({
id: z
Expand Down
56 changes: 54 additions & 2 deletions apps/server/test/assisted-update-migrations.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,9 +75,11 @@ describe("buildAssistedUpdateContext (migrations path)", () => {
);
expect(ctx.state.migrations?.map((m) => m.id)).toEqual(["first", "second"]);
expect(ctx.state.metadata).toBeNull();
// Union: service_entrypoint from first + health_endpoint from second
// Union: service_entrypoint from first + health_endpoint from second,
// plus the framework-enforced running_version.
expect(ctx.state.requiredChecks.sort()).toEqual([
"health_endpoint",
"running_version",
"service_entrypoint",
]);
expect(ctx.state.phase).toBe("inspect");
Expand DownExpand Up@@ -105,6 +107,7 @@ describe("buildAssistedUpdateContext (migrations path)", () => {
expect(ctx.state.requiredChecks).toEqual([
"health_endpoint",
"service_entrypoint",
"running_version",
]);
});

Expand DownExpand Up@@ -154,7 +157,10 @@ describe("buildAssistedUpdateContext (migrations path)", () => {
);
expect(ctx.state.migrations).toBeNull();
expect(ctx.state.metadata?.title).toBe("Legacy block");
expect(ctx.state.requiredChecks).toEqual(["health_endpoint"]);
expect(ctx.state.requiredChecks).toEqual([
"health_endpoint",
"running_version",
]);
expect(ctx.prompt).toContain("Legacy block");
// Legacy header, not the migration header
expect(ctx.prompt).not.toContain("Pending migrations");
Expand DownExpand Up@@ -260,4 +266,50 @@ describe("runAndRecordChecks (migrations path)", () => {
const state = await applied.readAppliedMigrationsState();
expect(state.appliedMigrations).toEqual({});
});

// A run launched by an older server has no running_version in its
// persisted requiredChecks — but the validating (newer) binary must still
// prove the running executable's version rather than trusting
// release.json.
it("enforces running_version even when the launched state predates it", async () => {
const { assisted } = await importEverything();
const ctx = await assisted.buildAssistedUpdateContext(
{
tag: "v0.18.13",
fromTag: "v0.18.12",
migrations: [MIGRATION("first")],
serverDir: "/tmp/dispatch-test-server",
recovery: RECOVERY,
},
"http://127.0.0.1:6767"
);
// Simulate state written by an older server version.
ctx.state.requiredChecks = ctx.state.requiredChecks.filter(
(name) => name !== "running_version"
);

const releaseChecks = await import("../src/release-checks.js");
let requestedNames: ReadonlyArray<string> = [];
const spy = vi
.spyOn(releaseChecks, "runRequiredChecks")
.mockImplementation(async (names) => {
requestedNames = names;
return names.map((name) => ({
name,
ok: true,
message: `${name} passed`,
}));
});

try {
await assisted.runAndRecordChecks(ctx.state, {
serverDir: "/tmp/dispatch-test-server",
targetTag: "v0.18.13",
});
} finally {
spy.mockRestore();
}

expect(requestedNames).toContain("running_version");
});
});
5 changes: 4 additions & 1 deletion apps/server/test/assisted-update.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -161,6 +161,7 @@ describe("buildAssistedUpdateContext", () => {
expect(ctx.state.requiredChecks).toEqual([
"service_restarted",
"version_converged",
"running_version",
]);
});

Expand DownExpand Up@@ -257,7 +258,9 @@ describe("buildAssistedUpdateContext", () => {
);
expect(ctx.prompt).not.toContain("## Instructions");
expect(ctx.prompt).not.toContain("## Rollback guidance");
expect(ctx.prompt).toContain("(none)");
// Even with no metadata checks, the framework-enforced running_version
// check is always listed.
expect(ctx.prompt).toContain("- running_version");
});
});

Expand Down
Loading
Loading