Skip to content

Commit 24d607a

Browse files
navedmerchantnavedmerchant
committed
address coderabbit comments
1 parent d580bf3 commit 24d607a

2 files changed

Lines changed: 100 additions & 17 deletions

File tree

‎src/services/code-index/semble/__tests__/semble-downloader.spec.ts‎

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,22 @@ import * as fs from "fs/promises"
33
import * as path from "path"
44
import { EventEmitter } from "events"
55

6+
// Mock crypto — verifyChecksum reads the archive file (mocked via createReadStream)
7+
// and computes a SHA-256. We make digest() dynamically return the expected checksum
8+
// for the current process.platform/arch so verification always passes in unit tests.
9+
const CHECKSUMS: Record<string, string> = {
10+
"linux-x64": "2bd4117dbd1ff7a26ed5ef44dad8d43162a4b9f431ec0bcc9dd2f9c6f5952e28",
11+
"linux-arm64": "177d14f41d3272594844a2635d59d97ad20400868a874a59169fd26a868c32a5",
12+
"darwin-arm64": "9130f447ff2c21803853a9aee58268f0e05134326384ac23d8b74ed22905e118",
13+
"win32-x64": "c8ae86f3703675e356824e08cf79c8a20c41c602296d2a5bff15bf35d762a46b",
14+
}
15+
vi.mock("crypto", () => ({
16+
createHash: vi.fn(() => ({
17+
update: vi.fn().mockReturnThis(),
18+
digest: vi.fn(() => CHECKSUMS[`${process.platform}-${process.arch}`] ?? "no-match"),
19+
})),
20+
}))
21+
622
// Mock fs/promises
723
vi.mock("fs/promises", () => ({
824
mkdir: vi.fn().mockResolvedValue(undefined),
@@ -14,27 +30,37 @@ vi.mock("fs/promises", () => ({
1430
writeFile: vi.fn().mockResolvedValue(undefined),
1531
}))
1632

17-
// Mock fs (createWriteStream)
33+
// Mock fs (createWriteStream and createReadStream for checksum verification)
1834
const mockWriteStream = {
1935
on: vi.fn(),
2036
close: vi.fn(),
2137
}
2238
vi.mock("fs", () => ({
2339
createWriteStream: vi.fn(() => mockWriteStream),
40+
createReadStream: vi.fn(() => {
41+
const { EventEmitter } = require("events")
42+
const stream = new EventEmitter()
43+
setImmediate(() => {
44+
stream.emit("data", Buffer.from("fake-archive-content"))
45+
stream.emit("end")
46+
})
47+
return stream
48+
}),
2449
}))
2550

26-
// Mock https
27-
const mockRequest = new EventEmitter() as any
28-
mockRequest.setTimeout = vi.fn()
29-
30-
const mockResponse = new EventEmitter() as any
31-
mockResponse.statusCode = 200
32-
mockResponse.headers = {}
33-
mockResponse.pipe = vi.fn()
34-
mockResponse.destroy = vi.fn()
51+
// Mock https — fresh emitters per invocation to avoid listener leaks across tests
52+
let mockRequest: any
53+
let mockResponse: any
3554

3655
vi.mock("https", () => ({
3756
get: vi.fn((_url: string, callback: (res: any) => void) => {
57+
mockRequest = Object.assign(new EventEmitter(), { setTimeout: vi.fn() })
58+
mockResponse = Object.assign(new EventEmitter(), {
59+
statusCode: 200,
60+
headers: {},
61+
pipe: vi.fn(),
62+
destroy: vi.fn(),
63+
})
3864
setImmediate(() => callback(mockResponse))
3965
return mockRequest
4066
}),
@@ -64,11 +90,6 @@ import { spawn } from "child_process"
6490
describe("semble-downloader", () => {
6591
beforeEach(() => {
6692
vi.clearAllMocks()
67-
// Reset response defaults
68-
mockResponse.statusCode = 200
69-
mockResponse.headers = {}
70-
mockResponse.pipe = vi.fn()
71-
mockResponse.destroy = vi.fn()
7293
mockWriteStream.on = vi.fn()
7394
mockWriteStream.close = vi.fn()
7495
})
@@ -252,7 +273,17 @@ describe("semble-downloader", () => {
252273
;(fs.readFile as any).mockRejectedValue(new Error("ENOENT"))
253274

254275
// Simulate HTTP error response
255-
mockResponse.statusCode = 404
276+
;(https.get as any).mockImplementation((_url: string, callback: (res: any) => void) => {
277+
const res = Object.assign(new EventEmitter(), {
278+
statusCode: 404,
279+
headers: {},
280+
pipe: vi.fn(),
281+
destroy: vi.fn(),
282+
})
283+
setImmediate(() => callback(res))
284+
const req = Object.assign(new EventEmitter(), { setTimeout: vi.fn() })
285+
return req
286+
})
256287

257288
try {
258289
await expect(downloadSemble("/storage")).rejects.toThrow("Failed to download semble")

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

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import * as fs from "fs/promises"
22
import * as path from "path"
33
import * as https from "https"
44
import { createWriteStream } from "fs"
5+
import { createHash } from "crypto"
6+
import { createReadStream } from "fs"
57
import { spawn } from "child_process"
68

79
/**
@@ -22,6 +24,40 @@ const SEMBLE_VERSION = "v0.3.1"
2224
const DOWNLOAD_BASE_URL = `https://github.com/navedmerchant/sembleexec/releases/download/${SEMBLE_VERSION}`
2325
const VERSION_FILE = ".semble-version"
2426

27+
/**
28+
* SHA-256 checksums for each platform archive at SEMBLE_VERSION.
29+
* These are verified after download to guard against tampered release assets.
30+
* Update these when bumping SEMBLE_VERSION.
31+
*
32+
* To regenerate: `shasum -a 256 <archive-file>`
33+
*/
34+
const SEMBLE_SHA256: Record<string, string> = {
35+
"linux-x64": "2bd4117dbd1ff7a26ed5ef44dad8d43162a4b9f431ec0bcc9dd2f9c6f5952e28",
36+
"linux-arm64": "177d14f41d3272594844a2635d59d97ad20400868a874a59169fd26a868c32a5",
37+
"darwin-arm64": "9130f447ff2c21803853a9aee58268f0e05134326384ac23d8b74ed22905e118",
38+
"win32-x64": "c8ae86f3703675e356824e08cf79c8a20c41c602296d2a5bff15bf35d762a46b",
39+
}
40+
41+
/**
42+
* Verifies the SHA-256 checksum of a downloaded file against the expected value.
43+
* Throws if the checksum does not match.
44+
*/
45+
export async function verifyChecksum(filePath: string, expected: string): Promise<void> {
46+
const hash = createHash("sha256")
47+
await new Promise<void>((resolve, reject) => {
48+
const stream = createReadStream(filePath)
49+
stream.on("data", (chunk) => hash.update(chunk))
50+
stream.on("end", resolve)
51+
stream.on("error", reject)
52+
})
53+
const actual = hash.digest("hex")
54+
if (actual !== expected) {
55+
throw new Error(
56+
`Checksum mismatch for ${path.basename(filePath)}: expected ${expected.slice(0, 12)}…, got ${actual.slice(0, 12)}…`,
57+
)
58+
}
59+
}
60+
2561
/**
2662
* Returns whether the current platform/arch has a prebuilt semble binary available.
2763
*/
@@ -127,6 +163,13 @@ export async function downloadSemble(storageDir: string): Promise<string | undef
127163
try {
128164
await downloadFile(url, archivePath)
129165

166+
// Verify archive integrity before extraction
167+
const platformKey = `${process.platform}-${process.arch}`
168+
const expectedChecksum = SEMBLE_SHA256[platformKey]
169+
if (expectedChecksum) {
170+
await verifyChecksum(archivePath, expectedChecksum)
171+
}
172+
130173
// Extract the archive
131174
await fs.mkdir(extractDir, { recursive: true })
132175

@@ -215,6 +258,15 @@ function extractTarGz(archivePath: string, destDir: string): Promise<void> {
215258
})
216259
}
217260

261+
/**
262+
* Escapes a string for use inside a PowerShell single-quoted literal.
263+
* In PowerShell, the only special character in a single-quoted string is the
264+
* apostrophe itself, which is escaped by doubling it.
265+
*/
266+
function escapePowerShellLiteral(value: string): string {
267+
return value.replace(/'/g, "''")
268+
}
269+
218270
/**
219271
* Extracts a .zip archive into the destination directory.
220272
* Uses PowerShell on Windows, unzip on other platforms.
@@ -229,7 +281,7 @@ function extractZip(archivePath: string, destDir: string): Promise<void> {
229281
[
230282
"-NoProfile",
231283
"-Command",
232-
`Expand-Archive -Path '${archivePath}' -DestinationPath '${destDir}' -Force`,
284+
`Expand-Archive -Path '${escapePowerShellLiteral(archivePath)}' -DestinationPath '${escapePowerShellLiteral(destDir)}' -Force`,
233285
],
234286
{ shell: false, stdio: ["ignore", "pipe", "pipe"] },
235287
)

0 commit comments

Comments
 (0)