-
Notifications
You must be signed in to change notification settings - Fork 0
feat(security): verify binary SHA-256 checksum on download (B5) #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||||||||
| import fs from 'fs'; | ||||||||||||
| import path from 'path'; | ||||||||||||
| import os from 'os'; | ||||||||||||
| import crypto from 'crypto'; | ||||||||||||
| import axios from 'axios'; | ||||||||||||
| import { promisify } from 'util'; | ||||||||||||
| import stream from 'stream'; | ||||||||||||
|
|
@@ -124,6 +125,9 @@ export class BinaryManager { | |||||||||||
| const writer = fs.createWriteStream(tempFilePath); | ||||||||||||
| await pipeline(response.data, writer); | ||||||||||||
|
|
||||||||||||
| // Verify checksum integrity | ||||||||||||
| await this.verifyChecksum(tempFilePath, assetName); | ||||||||||||
|
|
||||||||||||
| // Move to install dir | ||||||||||||
| // We rename it to capiscio-core (or .exe) for internal consistency | ||||||||||||
| fs.copyFileSync(tempFilePath, this.binaryPath); | ||||||||||||
|
|
@@ -156,6 +160,44 @@ export class BinaryManager { | |||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| private async verifyChecksum(filePath: string, assetName: string): Promise<void> { | ||||||||||||
| const checksumsUrl = `https://github.com/${REPO_OWNER}/${REPO_NAME}/releases/download/${VERSION}/checksums.txt`; | ||||||||||||
|
|
||||||||||||
| let expectedHash: string | null = null; | ||||||||||||
| try { | ||||||||||||
| const resp = await axios.get(checksumsUrl, { timeout: 30000 }); | ||||||||||||
| const lines = (resp.data as string).trim().split('\n'); | ||||||||||||
| for (const line of lines) { | ||||||||||||
| const parts = line.trim().split(/\s+/); | ||||||||||||
| if (parts.length === 2 && parts[1] === assetName) { | ||||||||||||
| expectedHash = parts[0] ?? null; | ||||||||||||
| break; | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
| } catch { | ||||||||||||
| console.warn('Warning: Could not fetch checksums.txt. Skipping integrity verification.'); | ||||||||||||
| return; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if (!expectedHash) { | ||||||||||||
| console.warn(`Warning: Asset ${assetName} not found in checksums.txt. Skipping verification.`); | ||||||||||||
| return; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const fileBuffer = fs.readFileSync(filePath); | ||||||||||||
| const actualHash = crypto.createHash('sha256').update(fileBuffer).digest('hex'); | ||||||||||||
|
Comment on lines
+187
to
+188
|
||||||||||||
| const fileBuffer = fs.readFileSync(filePath); | |
| const actualHash = crypto.createHash('sha256').update(fileBuffer).digest('hex'); | |
| const hash = crypto.createHash('sha256'); | |
| await pipeline(fs.createReadStream(filePath), hash); | |
| const actualHash = hash.digest('hex'); |
Copilot
AI
Mar 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checksum verification introduces new security-critical behavior (successful verification, mismatch error + deletion, and fallback when checksums.txt is missing), but there are no unit tests asserting these paths. Since this repo already has BinaryManager install tests, please add coverage by mocking axios.get for both the binary and checksums.txt, and mocking fs.readFileSync/hashing to simulate match/mismatch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
verifyChecksumthrows (e.g., checksum mismatch), the temp directory created for the download is not cleaned up because cleanup only happens after the copy step. Consider scopingtempDir/tempFilePathoutside the try block and using afinallytormSync(tempDir, { recursive: true, force: true })on both success and failure.