fix: address managed binary review feedback

Author: navedmerchant

src/services/code-index/semble/semble-downloader.ts

@@ -26,6 +26,7 @@ const SEMBLE_ARCHIVES: Record<string, { archive: string; binary: string }> = {
 export const SEMBLE_VERSION = "v0.4.1"
 const DOWNLOAD_BASE_URL = `https://github.com/Zoo-Code-Org/sembleexec/releases/download/${SEMBLE_VERSION}`
 const VERSION_FILE = ".semble-version"
+const MAX_ARCHIVE_BYTES = 50 * 1024 * 1024
 
 /**
  * SHA-256 checksums for each platform archive at SEMBLE_VERSION.
@@ -121,13 +122,16 @@ export async function downloadSemble(storageDir: string): Promise<string | undef
 				name: "Semble",
 				trustedDomains: TRUSTED_DOWNLOAD_DOMAINS,
 				timeoutMs: 120_000,
+				maxBytes: MAX_ARCHIVE_BYTES,
 			}),
 		verifyArchive: (archivePath) => verifyChecksum(archivePath, expectedChecksum),
 		extractArchive: async (archivePath, stagingDir) => {
 			if (info.archive.endsWith(".tar.gz")) {
 				await extractTarGzArchive(archivePath, stagingDir)
 			} else if (info.archive.endsWith(".zip")) {
 				await extractZipArchive(archivePath, stagingDir)
+			} else {
+				throw new Error(`Unsupported semble archive format: ${info.archive}`)
 			}
 		},
 	})

src/services/managed-binary/__tests__/archive.spec.ts

@@ -44,15 +44,18 @@ describe("managed binary archive utilities", () => {
 
 	it("kills a process that exceeds its timeout", async () => {
 		vi.useFakeTimers()
-		const child = createChild()
-		mockSpawn.mockReturnValue(child as unknown as ReturnType<typeof spawn>)
-		const processResult = runProcess("tool", [], 100)
-		const assertion = expect(processResult).rejects.toThrow("tool timed out")
-
-		await vi.advanceTimersByTimeAsync(100)
-		await assertion
-		expect(child.kill).toHaveBeenCalledWith("SIGKILL")
-		vi.useRealTimers()
+		try {
+			const child = createChild()
+			mockSpawn.mockReturnValue(child as unknown as ReturnType<typeof spawn>)
+			const processResult = runProcess("tool", [], 100)
+			const assertion = expect(processResult).rejects.toThrow("tool timed out")
+
+			await vi.advanceTimersByTimeAsync(100)
+			await assertion
+			expect(child.kill).toHaveBeenCalledWith("SIGKILL")
+		} finally {
+			vi.useRealTimers()
+		}
 	})
 
 	it("extracts tar.gz archives with hardened flags", async () => {
@@ -112,7 +115,7 @@ describe("managed binary archive utilities", () => {
 		mockSpawn.mockReturnValueOnce(listing as unknown as ReturnType<typeof spawn>)
 		mockSpawn.mockReturnValueOnce(extraction as unknown as ReturnType<typeof spawn>)
 		const result = extractSingleFileTarXzArchive("/tmp/archive.tar.xz", "/tmp/output", "binary", "Tool")
-		listing.stdout.write("./binary\n")
+		listing.stdout.write("-rwxr-xr-x user/group 1 2026-01-01 00:00 ./binary\n")
 		listing.emit("close", 0)
 		await new Promise<void>((resolve) => setImmediate(resolve))
 		extraction.emit("close", 0)
@@ -121,11 +124,36 @@ describe("managed binary archive utilities", () => {
 		expect(mockSpawn).toHaveBeenNthCalledWith(
 			2,
 			"tar",
-			["-xJf", "/tmp/archive.tar.xz", "-C", "/tmp/output", "./binary"],
+			[
+				"-xJf",
+				"/tmp/archive.tar.xz",
+				"-C",
+				"/tmp/output",
+				"--no-same-owner",
+				...(process.platform === "linux" ? ["--no-overwrite-dir"] : []),
+				"./binary",
+			],
 			expect.any(Object),
 		)
 	})
 
+	it.each([
+		["-rwxr-xr-x user/group 1 2026-01-01 00:00 ./other\n", "an unexpected filename"],
+		[
+			"-rwxr-xr-x user/group 1 2026-01-01 00:00 ./binary\n-rwxr-xr-x user/group 1 2026-01-01 00:00 ./other\n",
+			"multiple entries",
+		],
+		["lrwxrwxrwx user/group 0 2026-01-01 00:00 ./binary\n", "a non-regular entry"],
+	])("rejects a tar.xz archive with %s", async (listingOutput) => {
+		const listing = createChild()
+		mockSpawn.mockReturnValue(listing as unknown as ReturnType<typeof spawn>)
+		const result = extractSingleFileTarXzArchive("/tmp/archive.tar.xz", "/tmp/output", "binary", "Tool")
+		listing.stdout.write(listingOutput)
+		listing.emit("close", 0)
+
+		await expect(result).rejects.toThrow("Tool archive has an unexpected layout")
+	})
+
 	it("builds a single-entry-validated PowerShell ZIP extraction", async () => {
 		const child = createChild()
 		mockSpawn.mockReturnValue(child as unknown as ReturnType<typeof spawn>)

src/services/managed-binary/__tests__/download.spec.ts

@@ -104,6 +104,16 @@ describe("managed binary downloads", () => {
 		await expect(verification).rejects.toThrow("checksum mismatch: actual-checksum")
 	})
 
+	it("accepts a matching SHA-256 checksum", async () => {
+		const input = new EventEmitter()
+		mockCreateReadStream.mockReturnValue(input as ReturnType<typeof createReadStream>)
+		const verification = verifySha256Checksum("/tmp/archive", "actual-checksum", () => new Error("should not fail"))
+		input.emit("data", Buffer.from("archive"))
+		input.emit("end")
+
+		await expect(verification).resolves.toBeUndefined()
+	})
+
 	it("follows a trusted redirect and applies destination security options", async () => {
 		const requestOne = createRequest()
 		const requestTwo = createRequest()

src/services/managed-binary/__tests__/install.spec.ts

@@ -68,7 +68,10 @@ describe("managed binary installation", () => {
 		expect(first).toBe(second)
 		await vi.waitFor(() => expect(download).toHaveBeenCalledOnce())
 		finishDownload?.()
-		await Promise.all([first, second])
+		await expect(Promise.all([first, second])).resolves.toEqual([
+			getManagedBinaryPaths(options).binaryPath,
+			getManagedBinaryPaths(options).binaryPath,
+		])
 		expect(download).toHaveBeenCalledOnce()
 	})
 

src/services/managed-binary/archive.ts

@@ -70,6 +70,7 @@ export async function extractSingleFileZipArchive(
 	expectedFile: string,
 	archiveName: string,
 ): Promise<void> {
+	// This deliberately uses PowerShell because it is only called for Windows release archives.
 	const script = [
 		"$ErrorActionPreference = 'Stop'",
 		"$archivePath = $args[0]",
@@ -103,15 +104,21 @@ export async function extractSingleFileTarXzArchive(
 	expectedFile: string,
 	archiveName: string,
 ): Promise<void> {
-	const listing = await runProcess("tar", ["-tJf", archivePath])
+	const listing = await runProcess("tar", ["-tvJf", archivePath])
 	const entries = listing.stdout
 		.split(/\r?\n/)
 		.map((entry) => entry.trim())
 		.filter(Boolean)
 	const archiveEntry = entries[0]
-	if (entries.length !== 1 || archiveEntry.replace(/^\.\//, "") !== expectedFile) {
+	const entryName = archiveEntry?.split(/\s+/).at(-1)
+	if (entries.length !== 1 || !archiveEntry.startsWith("-") || entryName?.replace(/^\.\//, "") !== expectedFile) {
 		throw new Error(`${archiveName} archive has an unexpected layout`)
 	}
 
-	await runProcess("tar", ["-xJf", archivePath, "-C", destination, archiveEntry])
+	const args = ["-xJf", archivePath, "-C", destination, "--no-same-owner"]
+	if (process.platform === "linux") {
+		args.push("--no-overwrite-dir")
+	}
+	args.push(entryName)
+	await runProcess("tar", args)
 }

src/services/managed-binary/download.ts

@@ -125,27 +125,30 @@ function downloadBinaryFileWithRedirects(
 				destination,
 				options.exclusiveDestination ? { flags: "wx", mode: 0o600 } : undefined,
 			)
+			const abort = (error: Error) => {
+				response.unpipe(output)
+				output.destroy()
+				response.destroy()
+				request.destroy()
+				reject(error)
+			}
 			response.on("data", (chunk: Buffer) => {
 				received += chunk.length
 				if (options.maxBytes !== undefined) {
 					try {
 						assertSizeWithinLimit(received, options.maxBytes, options.name)
 					} catch (error) {
-						response.unpipe(output)
-						output.destroy()
-						response.destroy()
-						request.destroy()
-						reject(error)
+						abort(error instanceof Error ? error : new Error(String(error)))
 					}
 				}
 			})
-			response.on("error", reject)
+			response.on("error", abort)
 			response.pipe(output)
 			output.on("finish", () => {
 				output.close()
 				resolve()
 			})
-			output.on("error", reject)
+			output.on("error", abort)
 		})
 
 		request.setTimeout(options.timeoutMs, () => request.destroy(new Error(`${options.name} download timed out`)))

src/services/managed-binary/install.ts

@@ -124,6 +124,9 @@ async function installManagedBinaryWithLock(options: ManagedBinaryInstallOptions
 		stale: 5 * 60_000,
 		update: 30_000,
 		retries: { retries: 10, factor: 1.5, minTimeout: 100, maxTimeout: 1_000 },
+		onCompromised: (error) => {
+			throw error
+		},
 	})
 	try {
 		return await installManagedBinary(options)