diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 7c9fcf50c..f6579dd36 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed direct binary updates trusting an executable that only reported the expected version. The updater now selects one exact asset from the tagged GitHub release, requires its published SHA-256 digest and size, and verifies both while streaming the download before installation. + ## [17.1.3] - 2026-07-24 ### Fixed diff --git a/packages/coding-agent/src/cli/update-cli.ts b/packages/coding-agent/src/cli/update-cli.ts index 59cfdfbf2..09073a487 100644 --- a/packages/coding-agent/src/cli/update-cli.ts +++ b/packages/coding-agent/src/cli/update-cli.ts @@ -4,9 +4,11 @@ * Handles `omp update` to check for and install updates. * Uses the installer that owns the active omp executable when it can be detected. */ +import { createHash } from "node:crypto"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; +import { Transform } from "node:stream"; import { pipeline } from "node:stream/promises"; import { $which, APP_NAME, isEnoent, VERSION } from "@oh-my-pi/pi-utils"; import { $ } from "bun"; @@ -30,6 +32,7 @@ const MISE_TOOL = "github:can1357/oh-my-pi"; * See #1686. */ const NPM_REGISTRY = "https://registry.npmjs.org/"; +const GITHUB_API = "https://api.github.com"; const RELEASE_METADATA_TIMEOUT_MS = 30_000; const BINARY_DOWNLOAD_TIMEOUT_MS = 15 * 60_000; @@ -65,6 +68,164 @@ interface ReleaseInfo { version: string; } +export interface ReleaseBinaryAsset { + url: string; + size: number; + digest: string; +} + +type Fetch = (input: string | URL | Request, init?: RequestInit) => Promise; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null; +} + +/** + * Select and validate the binary asset from GitHub release metadata. + */ +export function resolveReleaseBinaryAsset( + release: unknown, + expectedTag: string, + binaryName: string, +): ReleaseBinaryAsset { + if (!isRecord(release)) { + throw new Error("Invalid GitHub release metadata"); + } + if (release.tag_name !== expectedTag) { + throw new Error(`GitHub release tag mismatch: expected ${expectedTag}`); + } + if (release.draft !== false || release.prerelease !== false) { + throw new Error(`GitHub release ${expectedTag} is not a published stable release`); + } + if (!Array.isArray(release.assets)) { + throw new Error(`GitHub release ${expectedTag} has no asset list`); + } + + const matches = release.assets.filter(asset => isRecord(asset) && asset.name === binaryName); + if (matches.length !== 1) { + throw new Error(`GitHub release ${expectedTag} has ${matches.length} assets named ${binaryName}`); + } + + const asset = matches[0]; + if (!isRecord(asset) || asset.state !== "uploaded") { + throw new Error(`GitHub release asset ${binaryName} is not fully uploaded`); + } + if (typeof asset.size !== "number" || !Number.isSafeInteger(asset.size) || asset.size <= 0) { + throw new Error(`GitHub release asset ${binaryName} has an invalid size`); + } + if (typeof asset.digest !== "string") { + throw new Error(`GitHub release asset ${binaryName} has no digest`); + } + const digest = /^sha256:([0-9a-f]{64})$/i.exec(asset.digest)?.[1]; + if (!digest) { + throw new Error(`GitHub release asset ${binaryName} has an unsupported digest`); + } + + const expectedUrl = `https://github.com/${REPO}/releases/download/${expectedTag}/${binaryName}`; + if (asset.browser_download_url !== expectedUrl) { + throw new Error(`GitHub release asset ${binaryName} has an unexpected download URL`); + } + + return { + url: expectedUrl, + size: asset.size, + digest: `sha256:${digest.toLowerCase()}`, + }; +} + +async function getReleaseBinaryAsset( + expectedVersion: string, + binaryName: string, + fetchImpl: Fetch = fetch, +): Promise { + const tag = `v${expectedVersion}`; + let response: Response; + try { + response = await fetchImpl(`${GITHUB_API}/repos/${REPO}/releases/tags/${encodeURIComponent(tag)}`, { + headers: { + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + }, + signal: withTimeoutSignal(RELEASE_METADATA_TIMEOUT_MS), + }); + } catch (err) { + if (isTimeoutError(err)) { + throw new Error("Timed out fetching GitHub release metadata after 30s", { cause: err }); + } + throw err; + } + if (!response.ok) { + throw new Error(`Failed to fetch GitHub release metadata: ${response.statusText}`); + } + + return resolveReleaseBinaryAsset(await response.json(), tag, binaryName); +} + +export interface VerifiedBinaryDownloadOptions { + url: string; + targetPath: string; + expectedSize: number; + expectedDigest: string; + fetchImpl?: Fetch; +} + +/** + * Download a binary and verify its GitHub-reported size and SHA-256 digest. + */ +export async function downloadVerifiedBinary(options: VerifiedBinaryDownloadOptions): Promise { + const fetchImpl = options.fetchImpl ?? fetch; + await unlinkIfExists(options.targetPath); + + let response: Response; + try { + response = await fetchImpl(options.url, { + redirect: "follow", + signal: withTimeoutSignal(BINARY_DOWNLOAD_TIMEOUT_MS), + }); + } catch (err) { + if (isTimeoutError(err)) { + throw new Error("Timed out downloading release binary after 15 minutes", { cause: err }); + } + throw err; + } + if (!response.ok || !response.body) { + throw new Error(`Download failed: ${response.statusText}`); + } + + const hash = createHash("sha256"); + let size = 0; + const verifier = new Transform({ + transform(chunk, _encoding, callback) { + size += chunk.byteLength; + if (size > options.expectedSize) { + callback( + new Error( + `Downloaded binary size mismatch: expected ${options.expectedSize} bytes, received at least ${size}`, + ), + ); + return; + } + hash.update(chunk); + callback(null, chunk); + }, + }); + + try { + await pipeline(response.body, verifier, fs.createWriteStream(options.targetPath, { mode: 0o600 })); + const digest = `sha256:${hash.digest("hex")}`; + if (size !== options.expectedSize) { + throw new Error(`Downloaded binary size mismatch: expected ${options.expectedSize} bytes, received ${size}`); + } + if (digest !== options.expectedDigest) { + throw new Error(`Downloaded binary digest mismatch: expected ${options.expectedDigest}, received ${digest}`); + } + await fs.promises.chmod(options.targetPath, 0o755); + } catch (err) { + await unlinkIfExists(options.targetPath); + throw err; + } +} + /** Result from running the installed binary and parsing its reported version. */ export interface InstalledVersionVerification { ok: boolean; @@ -895,36 +1056,32 @@ async function updateViaMise(expectedVersion: string, force: boolean): Promise { - const binaryName = getBinaryName(); - const tag = `v${expectedVersion}`; - const url = `https://github.com/${REPO}/releases/download/${tag}/${binaryName}`; - +export async function updateViaBinaryAt( + targetPath: string, + expectedVersion: string, + options: { + binaryName?: string; + fetchImpl?: Fetch; + verifyInstalledVersion?: typeof verifyInstalledVersion; + } = {}, +): Promise { + const binaryName = options.binaryName ?? getBinaryName(); const tempPath = `${targetPath}.new`; // Unique per attempt: a stale backup from an earlier update may still be // locked (it is the previous process image on Windows), and a fixed name // would force the move-aside rename to overwrite it. pid + timestamp keeps // two forced updates in the same millisecond from colliding. const backupPath = `${targetPath}.${Date.now()}.${process.pid}.bak`; + const asset = await getReleaseBinaryAsset(expectedVersion, binaryName, options.fetchImpl); console.log(chalk.dim(`Downloading ${binaryName}…`)); - - let response: Response; - try { - response = await fetch(url, { - redirect: "follow", - signal: withTimeoutSignal(BINARY_DOWNLOAD_TIMEOUT_MS), - }); - } catch (err) { - if (isTimeoutError(err)) { - throw new Error("Timed out downloading release binary after 15 minutes", { cause: err }); - } - throw err; - } - if (!response.ok || !response.body) { - throw new Error(`Download failed: ${response.statusText}`); - } - const fileStream = fs.createWriteStream(tempPath, { mode: 0o755 }); - await pipeline(response.body, fileStream); + await downloadVerifiedBinary({ + url: asset.url, + targetPath: tempPath, + expectedSize: asset.size, + expectedDigest: asset.digest, + fetchImpl: options.fetchImpl, + }); + console.log(chalk.dim(`Verified ${asset.digest}`)); console.log(chalk.dim("Installing update...")); await replaceBinaryForUpdate({ @@ -932,7 +1089,7 @@ async function updateViaBinaryAt(targetPath: string, expectedVersion: string): P tempPath, backupPath, expectedVersion, - verifyInstalledVersion, + verifyInstalledVersion: options.verifyInstalledVersion ?? verifyInstalledVersion, }); // Reclaim backups from earlier updates whose owning process has since exited. await sweepStaleBackups(targetPath); diff --git a/packages/coding-agent/test/update-cli.test.ts b/packages/coding-agent/test/update-cli.test.ts index 99f93c299..b0c81bc0e 100644 --- a/packages/coding-agent/test/update-cli.test.ts +++ b/packages/coding-agent/test/update-cli.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it, spyOn, vi } from "bun:test"; +import { createHash } from "node:crypto"; import * as nodeFs from "node:fs"; import * as fs from "node:fs/promises"; import * as os from "node:os"; @@ -11,12 +12,15 @@ import { buildMiseForceInstallArgs, buildMiseUpgradeArgs, buildNpmInstallArgs, + downloadVerifiedBinary, parseUpdateArgs, pruneBunInstallCache, replaceBinaryForUpdate, resolveBunGlobalNodeModulesDirFromLocations, + resolveReleaseBinaryAsset, resolveUpdateMethodForTest, sweepStaleBackups, + updateViaBinaryAt, } from "@oh-my-pi/pi-coding-agent/cli/update-cli"; import Update from "@oh-my-pi/pi-coding-agent/commands/update"; import { removeWithRetries } from "@oh-my-pi/pi-utils"; @@ -32,6 +36,7 @@ async function makeTempDir(): Promise { afterEach(async () => { vi.restoreAllMocks(); + await Promise.all(tempDirs.splice(0).map(dir => removeWithRetries(dir))); }); const TEST_CONFIG: CliConfig = { @@ -303,6 +308,177 @@ describe("update-cli bun cache pruning", () => { }); }); +describe("update-cli release binary integrity", () => { + const tag = "v17.1.2"; + const binaryName = "omp-linux-x64"; + const url = `https://github.com/can1357/oh-my-pi/releases/download/${tag}/${binaryName}`; + const content = "verified binary"; + const digest = `sha256:${createHash("sha256").update(content).digest("hex")}`; + + function releaseAsset(overrides: Record = {}): Record { + return { + tag_name: tag, + draft: false, + prerelease: false, + assets: [ + { + name: binaryName, + state: "uploaded", + size: Buffer.byteLength(content), + digest, + browser_download_url: url, + ...overrides, + }, + ], + }; + } + + it("selects an uploaded asset with a valid SHA-256 digest", () => { + expect(resolveReleaseBinaryAsset(releaseAsset(), tag, binaryName)).toEqual({ + url, + size: Buffer.byteLength(content), + digest, + }); + }); + + it("rejects missing and unsupported release asset digests", () => { + expect(() => resolveReleaseBinaryAsset(releaseAsset({ digest: null }), tag, binaryName)).toThrow("has no digest"); + expect(() => resolveReleaseBinaryAsset(releaseAsset({ digest: "sha512:abc" }), tag, binaryName)).toThrow( + "has an unsupported digest", + ); + }); + + it("rejects release metadata that does not identify one exact stable asset", () => { + expect(() => resolveReleaseBinaryAsset({ ...releaseAsset(), prerelease: true }, tag, binaryName)).toThrow( + "is not a published stable release", + ); + expect(() => resolveReleaseBinaryAsset({ ...releaseAsset(), assets: [] }, tag, binaryName)).toThrow( + `has 0 assets named ${binaryName}`, + ); + expect(() => + resolveReleaseBinaryAsset( + { ...releaseAsset(), assets: [releaseAsset().assets, releaseAsset().assets].flat() }, + tag, + binaryName, + ), + ).toThrow(`has 2 assets named ${binaryName}`); + expect(() => + resolveReleaseBinaryAsset( + releaseAsset({ browser_download_url: "https://example.com/omp-linux-x64" }), + tag, + binaryName, + ), + ).toThrow("has an unexpected download URL"); + }); + + it("writes a download only after its size and digest match", async () => { + const dir = await makeTempDir(); + const targetPath = path.join(dir, binaryName); + + await downloadVerifiedBinary({ + url, + targetPath, + expectedSize: Buffer.byteLength(content), + expectedDigest: digest, + fetchImpl: async () => new Response(content), + }); + + expect(await Bun.file(targetPath).text()).toBe(content); + expect((await fs.stat(targetPath)).mode & 0o777).toBe(0o755); + }); + + it("aborts the response stream as soon as it exceeds the expected size", async () => { + const dir = await makeTempDir(); + const targetPath = path.join(dir, binaryName); + let pulls = 0; + const body = new ReadableStream( + { + pull(controller) { + pulls++; + controller.enqueue(new Uint8Array(pulls === 1 ? 2 : 1)); + if (pulls === 2) controller.close(); + }, + }, + { highWaterMark: 0 }, + ); + + await expect( + downloadVerifiedBinary({ + url, + targetPath, + expectedSize: 1, + expectedDigest: digest, + fetchImpl: async () => new Response(body), + }), + ).rejects.toThrow("received at least 2"); + expect(pulls).toBe(1); + expect(await Bun.file(targetPath).exists()).toBe(false); + }); + + it("removes downloads whose size or digest does not match", async () => { + const dir = await makeTempDir(); + const targetPath = path.join(dir, binaryName); + const fetchImpl = async () => new Response(content); + + await expect( + downloadVerifiedBinary({ + url, + targetPath, + expectedSize: Buffer.byteLength(content) + 1, + expectedDigest: digest, + fetchImpl, + }), + ).rejects.toThrow("size mismatch"); + expect(await Bun.file(targetPath).exists()).toBe(false); + + await expect( + downloadVerifiedBinary({ + url, + targetPath, + expectedSize: Buffer.byteLength(content), + expectedDigest: `sha256:${createHash("sha256").update("different binary").digest("hex")}`, + fetchImpl, + }), + ).rejects.toThrow("digest mismatch"); + expect(await Bun.file(targetPath).exists()).toBe(false); + }); + + it("rejects an altered version-reporting executable before replacing the installed binary", async () => { + const dir = await makeTempDir(); + const targetPath = path.join(dir, binaryName); + const installed = "#!/bin/sh\necho omp/17.0.8\n"; + const altered = "#!/bin/sh\necho omp/17.1.2\n"; + const expectedDigest = `sha256:${createHash("sha256") + .update("x".repeat(Buffer.byteLength(altered))) + .digest("hex")}`; + await Bun.write(targetPath, installed); + await fs.chmod(targetPath, 0o755); + + const fetchImpl = async (input: string | URL | Request): Promise => { + const requestUrl = String(input); + if (requestUrl.startsWith("https://api.github.com/")) { + return new Response( + JSON.stringify( + releaseAsset({ + size: Buffer.byteLength(altered), + digest: expectedDigest, + }), + ), + ); + } + if (requestUrl === url) return new Response(altered); + throw new Error(`Unexpected request: ${requestUrl}`); + }; + + await expect(updateViaBinaryAt(targetPath, "17.1.2", { binaryName, fetchImpl })).rejects.toThrow( + "digest mismatch", + ); + expect(await Bun.file(targetPath).text()).toBe(installed); + expect((await fs.stat(targetPath)).mode & 0o777).toBe(0o755); + expect(await Bun.file(`${targetPath}.new`).exists()).toBe(false); + }); +}); + describe("update-cli binary replacement", () => { it("restores the previous binary when the replacement fails verification", async () => { const dir = await makeTempDir();