From 028147b631c56696a099c5e3ebf3d80b39ebb246 Mon Sep 17 00:00:00 2001 From: Nikola Irinchev Date: Thu, 13 Mar 2025 16:07:23 +0100 Subject: [PATCH 1/3] chore(releasing): validate tarball sha1 before publishing to homebrew --- .evergreen/verify-packaged-artifact.sh | 9 +- .../src/homebrew/publish-to-homebrew.spec.ts | 28 +++--- .../build/src/homebrew/publish-to-homebrew.ts | 12 +-- packages/build/src/homebrew/utils.spec.ts | 91 ++++++++++++++++++- packages/build/src/homebrew/utils.ts | 65 ++++++++++++- 5 files changed, 171 insertions(+), 34 deletions(-) diff --git a/.evergreen/verify-packaged-artifact.sh b/.evergreen/verify-packaged-artifact.sh index 0fdbadf79b..5f187ea8b8 100755 --- a/.evergreen/verify-packaged-artifact.sh +++ b/.evergreen/verify-packaged-artifact.sh @@ -31,6 +31,13 @@ verify_using_gpg() { verify_using_powershell() { echo "Verifying $1 using powershell" powershell Get-AuthenticodeSignature -FilePath $ARTIFACTS_DIR/$1 > "$TMP_FILE" 2>&1 + + # Get-AuthenticodeSignature just outputs text, it doesn't exit with a non-zero + # code if the file is not signed + if grep -q NotSigned "$TMP_FILE"; then + echo "File $1 is not signed" + exit 1 + fi } verify_using_codesign() { @@ -91,4 +98,4 @@ else (cd "$ARTIFACTS_DIR" && bash "$BASEDIR/retry-with-backoff.sh" curl -sSfLO --url "$(cat "$ARTIFACT_URL_FILE").sig") verify_using_gpg $ARTIFACT_FILE_NAME fi -fi \ No newline at end of file +fi diff --git a/packages/build/src/homebrew/publish-to-homebrew.spec.ts b/packages/build/src/homebrew/publish-to-homebrew.spec.ts index c1ed2be1fd..2874dc7155 100644 --- a/packages/build/src/homebrew/publish-to-homebrew.spec.ts +++ b/packages/build/src/homebrew/publish-to-homebrew.spec.ts @@ -10,7 +10,7 @@ describe('HomebrewPublisher', function () { let homebrewCore: GithubRepo; let homebrewCoreFork: GithubRepo; let createPullRequest: sinon.SinonStub; - let httpsSha256: sinon.SinonStub; + let npmPackageSha256: sinon.SinonStub; let generateFormula: sinon.SinonStub; let updateHomebrewFork: sinon.SinonStub; @@ -41,7 +41,7 @@ describe('HomebrewPublisher', function () { homebrewCoreFork, }); - httpsSha256 = sinon.stub(testPublisher, 'httpsSha256'); + npmPackageSha256 = sinon.stub(testPublisher, 'npmPackageSha256'); generateFormula = sinon.stub(testPublisher, 'generateFormula'); updateHomebrewFork = sinon.stub(testPublisher, 'updateHomebrewFork'); }; @@ -61,11 +61,9 @@ describe('HomebrewPublisher', function () { isDryRun: false, }); - httpsSha256 + npmPackageSha256 .rejects() - .withArgs( - 'https://registry.npmjs.org/@mongosh/cli-repl/-/cli-repl-1.0.0.tgz' - ) + .withArgs('https://registry.npmjs.org/@mongosh/cli-repl/1.0.0') .resolves('sha'); generateFormula @@ -97,7 +95,7 @@ describe('HomebrewPublisher', function () { await testPublisher.publish(); - expect(httpsSha256).to.have.been.called; + expect(npmPackageSha256).to.have.been.called; expect(generateFormula).to.have.been.called; expect(updateHomebrewFork).to.have.been.called; expect(createPullRequest).to.have.been.called; @@ -110,11 +108,9 @@ describe('HomebrewPublisher', function () { isDryRun: false, }); - httpsSha256 + npmPackageSha256 .rejects() - .withArgs( - 'https://registry.npmjs.org/@mongosh/cli-repl/-/cli-repl-1.0.0.tgz' - ) + .withArgs('https://registry.npmjs.org/@mongosh/cli-repl/1.0.0') .resolves('sha'); generateFormula @@ -136,18 +132,16 @@ describe('HomebrewPublisher', function () { await testPublisher.publish(); - expect(httpsSha256).to.have.been.called; + expect(npmPackageSha256).to.have.been.called; expect(generateFormula).to.have.been.called; expect(updateHomebrewFork).to.have.been.called; expect(createPullRequest).to.not.have.been.called; }); it('silently ignores an error while deleting the PR branch', async function () { - httpsSha256 + npmPackageSha256 .rejects() - .withArgs( - 'https://registry.npmjs.org/@mongosh/cli-repl/-/cli-repl-1.0.0.tgz' - ) + .withArgs('https://registry.npmjs.org/@mongosh/cli-repl/1.0.0') .resolves('sha'); generateFormula @@ -179,7 +173,7 @@ describe('HomebrewPublisher', function () { await testPublisher.publish(); - expect(httpsSha256).to.have.been.called; + expect(npmPackageSha256).to.have.been.called; expect(generateFormula).to.have.been.called; expect(updateHomebrewFork).to.have.been.called; expect(createPullRequest).to.have.been.called; diff --git a/packages/build/src/homebrew/publish-to-homebrew.ts b/packages/build/src/homebrew/publish-to-homebrew.ts index e09e74601f..3ea5369c1c 100644 --- a/packages/build/src/homebrew/publish-to-homebrew.ts +++ b/packages/build/src/homebrew/publish-to-homebrew.ts @@ -1,7 +1,7 @@ import type { GithubRepo } from '@mongodb-js/devtools-github-repo'; import { generateUpdatedFormula as generateUpdatedFormulaFn } from './generate-formula'; import { updateHomebrewFork as updateHomebrewForkFn } from './update-homebrew-fork'; -import { httpsSha256 as httpsSha256Fn } from './utils'; +import { npmPackageSha256 as npmPackageSha256Fn } from './utils'; export type HomebrewPublisherConfig = { homebrewCore: GithubRepo; @@ -12,19 +12,19 @@ export type HomebrewPublisherConfig = { }; export class HomebrewPublisher { - readonly httpsSha256: typeof httpsSha256Fn; + readonly npmPackageSha256: typeof npmPackageSha256Fn; readonly generateFormula: typeof generateUpdatedFormulaFn; readonly updateHomebrewFork: typeof updateHomebrewForkFn; constructor( public config: HomebrewPublisherConfig, { - httpsSha256 = httpsSha256Fn, + npmPackageSha256 = npmPackageSha256Fn, generateFormula = generateUpdatedFormulaFn, updateHomebrewFork = updateHomebrewForkFn, } = {} ) { - this.httpsSha256 = httpsSha256; + this.npmPackageSha256 = npmPackageSha256; this.generateFormula = generateFormula; this.updateHomebrewFork = updateHomebrewFork; } @@ -38,10 +38,10 @@ export class HomebrewPublisher { githubReleaseLink, } = this.config; - const cliReplPackageUrl = `https://registry.npmjs.org/@mongosh/cli-repl/-/cli-repl-${packageVersion}.tgz`; + const cliReplPackageUrl = `https://registry.npmjs.org/@mongosh/cli-repl/${packageVersion}`; const packageSha = isDryRun ? `dryRun-fakesha256-${Date.now()}` - : await this.httpsSha256(cliReplPackageUrl); + : await this.npmPackageSha256(cliReplPackageUrl); const homebrewFormula = await this.generateFormula( { version: packageVersion, sha: packageSha }, diff --git a/packages/build/src/homebrew/utils.spec.ts b/packages/build/src/homebrew/utils.spec.ts index 9967688615..4cfe797844 100644 --- a/packages/build/src/homebrew/utils.spec.ts +++ b/packages/build/src/homebrew/utils.spec.ts @@ -1,15 +1,96 @@ import { expect } from 'chai'; -import { httpsSha256 } from './utils'; +import { npmPackageSha256 } from './utils'; +import sinon from 'sinon'; +import crypto from 'crypto'; describe('Homebrew utils', function () { - describe('httpsSha256', function () { + describe('npmPackageSha256', function () { it('computes the correct sha', async function () { - const url = - 'https://registry.npmjs.org/@mongosh/cli-repl/-/cli-repl-0.6.1.tgz'; + const url = 'https://registry.npmjs.org/@mongosh/cli-repl/0.6.1'; const expectedSha = '3721ea662cd3775373d4d70f7593993564563d9379704896478db1d63f6c8470'; - expect(await httpsSha256(url)).to.equal(expectedSha); + expect(await npmPackageSha256(url)).to.equal(expectedSha); + }); + + describe('when response sha mismatches', function () { + const fakeTarball = Buffer.from('mongosh-2.4.2.tgz'); + const fakeTarballShasum = crypto + .createHash('sha1') + .update(fakeTarball) + .digest('hex'); + + it('retries', async function () { + const httpGet = sinon.stub(); + httpGet + .withArgs( + 'https://registry.npmjs.org/@mongosh/cli-repl/2.4.2', + 'json' + ) + .resolves({ + dist: { + tarball: + 'https://registry.npmjs.org/@mongosh/cli-repl/-/cli-repl-2.4.2.tgz', + shasum: fakeTarballShasum, + }, + }); + + httpGet + .withArgs( + 'https://registry.npmjs.org/@mongosh/cli-repl/-/cli-repl-2.4.2.tgz', + 'binary' + ) + .onFirstCall() + .resolves(Buffer.from('mongosh-2.4.2-incomplete.tgz')) // Simulate incomplete/wrong binary download + .onSecondCall() + .resolves(fakeTarball); + + const sha = await npmPackageSha256( + 'https://registry.npmjs.org/@mongosh/cli-repl/2.4.2', + httpGet + ); + + expect(sha).to.equal( + crypto.createHash('sha256').update(fakeTarball).digest('hex') + ); + }); + + it('throws if retries are exhausted', async function () { + const httpGet = sinon.stub(); + httpGet + .withArgs( + 'https://registry.npmjs.org/@mongosh/cli-repl/2.4.2', + 'json' + ) + .resolves({ + dist: { + tarball: + 'https://registry.npmjs.org/@mongosh/cli-repl/-/cli-repl-2.4.2.tgz', + shasum: fakeTarballShasum, + }, + }); + + httpGet + .withArgs( + 'https://registry.npmjs.org/@mongosh/cli-repl/-/cli-repl-2.4.2.tgz', + 'binary' + ) + .resolves(Buffer.from('mongosh-2.4.2-incomplete.tgz')); // Simulate incomplete/wrong binary download + + const incompleteTarballShasum = crypto + .createHash('sha1') + .update(Buffer.from('mongosh-2.4.2-incomplete.tgz')) + .digest('hex'); + + const err = await npmPackageSha256( + 'https://registry.npmjs.org/@mongosh/cli-repl/2.4.2', + httpGet + ).catch((e) => e); + + expect(err.message).to.equal( + `shasum mismatch: expected '${fakeTarballShasum}', got '${incompleteTarballShasum}'` + ); + }); }); }); }); diff --git a/packages/build/src/homebrew/utils.ts b/packages/build/src/homebrew/utils.ts index b58537779f..66d7ccc246 100644 --- a/packages/build/src/homebrew/utils.ts +++ b/packages/build/src/homebrew/utils.ts @@ -1,13 +1,68 @@ import crypto from 'crypto'; import https from 'https'; -export function httpsSha256(url: string): Promise { - return new Promise((resolve, reject) => { +export async function npmPackageSha256( + packageUrl: string, + httpGetFn: typeof httpGet = httpGet +): Promise { + const json = await httpGetFn(packageUrl, 'json'); + const tarballUrl = json.dist.tarball; + const shasum = json.dist.shasum; + + const tarball = await getTarballWithRetries(tarballUrl, shasum, httpGetFn); + const hash = crypto.createHash('sha256'); + hash.update(tarball); + return hash.digest('hex'); +} + +async function getTarballWithRetries( + url: string, + shasum: string, + httpGetFn: typeof httpGet, + attempts = 3 +): Promise { + try { + const tarball = await httpGetFn(url, 'binary'); + const hash = crypto.createHash('sha1').update(tarball).digest('hex'); + if (hash !== shasum) { + throw new Error(`shasum mismatch: expected '${shasum}', got '${hash}'`); + } + + return tarball; + } catch (err) { + if (attempts === 0) { + throw err; + } + + return getTarballWithRetries(url, shasum, httpGetFn, attempts - 1); + } +} + +export function httpGet(url: string, response: 'json'): Promise; +export function httpGet(url: string, response: 'binary'): Promise; + +export async function httpGet( + url: string, + responseType: 'json' | 'binary' +): Promise { + const response = await new Promise((resolve, reject) => { https.get(url, (stream) => { - const hash = crypto.createHash('sha256'); + let data: string | Buffer[] = responseType === 'json' ? '' : []; stream.on('error', (err) => reject(err)); - stream.on('data', (chunk) => hash.update(chunk)); - stream.on('end', () => resolve(hash.digest('hex'))); + stream.on('data', (chunk) => { + if (typeof data === 'string') { + data += chunk; + } else { + data.push(chunk); + } + }); + stream.on('end', () => resolve(data)); }); }); + + if (typeof response === 'string') { + return JSON.parse(response); + } + + return Buffer.concat(response); } From f513b10ad55440c4b8205f2a86b7e94d7a686a85 Mon Sep 17 00:00:00 2001 From: Nikola Irinchev Date: Fri, 14 Mar 2025 12:54:55 +0100 Subject: [PATCH 2/3] set encoding for json responses --- packages/build/src/homebrew/utils.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/build/src/homebrew/utils.ts b/packages/build/src/homebrew/utils.ts index 66d7ccc246..9946221d0a 100644 --- a/packages/build/src/homebrew/utils.ts +++ b/packages/build/src/homebrew/utils.ts @@ -47,6 +47,10 @@ export async function httpGet( ): Promise { const response = await new Promise((resolve, reject) => { https.get(url, (stream) => { + if (responseType === 'json') { + stream.setEncoding('utf8'); + } + let data: string | Buffer[] = responseType === 'json' ? '' : []; stream.on('error', (err) => reject(err)); stream.on('data', (chunk) => { From 4e80cf75a8c0db15fbc5ddd8756bbb811eb67311 Mon Sep 17 00:00:00 2001 From: Nikola Irinchev Date: Thu, 20 Mar 2025 12:04:35 +0100 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Gagik Amaryan --- packages/build/src/homebrew/utils.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/build/src/homebrew/utils.ts b/packages/build/src/homebrew/utils.ts index 9946221d0a..00d7be8ed6 100644 --- a/packages/build/src/homebrew/utils.ts +++ b/packages/build/src/homebrew/utils.ts @@ -5,7 +5,12 @@ export async function npmPackageSha256( packageUrl: string, httpGetFn: typeof httpGet = httpGet ): Promise { - const json = await httpGetFn(packageUrl, 'json'); + const json = await httpGetFn<{ + dist: { + tarball: string; + shasum: string; + } + }>(packageUrl, 'json'); const tarballUrl = json.dist.tarball; const shasum = json.dist.shasum; @@ -38,13 +43,12 @@ async function getTarballWithRetries( } } -export function httpGet(url: string, response: 'json'): Promise; +export function httpGet(url: string, response: 'json'): Promise; export function httpGet(url: string, response: 'binary'): Promise; - -export async function httpGet( +export async function httpGet( url: string, responseType: 'json' | 'binary' -): Promise { +): Promise { const response = await new Promise((resolve, reject) => { https.get(url, (stream) => { if (responseType === 'json') {