Make our hash validation more strict. ALWAYS validate, even on cache hits.

Author: falahat

src/apphosting/universalMakerDownload.spec.ts

@@ -0,0 +1,84 @@
+import * as sinon from "sinon";
+import { expect } from "chai";
+import * as fs from "fs-extra";
+import * as path from "path";
+import * as downloadUtils from "../downloadUtils";
+import { getOrDownloadUniversalMaker } from "./universalMakerDownload";
+
+describe("universalMakerDownload", () => {
+  let existsSyncStub: sinon.SinonStub;
+  let ensureDirSyncStub: sinon.SinonStub;
+  let copySyncStub: sinon.SinonStub;
+  let chmodSyncStub: sinon.SinonStub;
+  let downloadToTmpStub: sinon.SinonStub;
+  let validateSizeStub: sinon.SinonStub;
+  let validateChecksumStub: sinon.SinonStub;
+
+  let originalPlatform: string;
+  let originalArch: string;
+
+  beforeEach(() => {
+    originalPlatform = process.platform;
+    originalArch = process.arch;
+    Object.defineProperty(process, "platform", { value: "linux", configurable: true });
+    Object.defineProperty(process, "arch", { value: "x64", configurable: true });
+
+    existsSyncStub = sinon.stub(fs, "existsSync");
+    ensureDirSyncStub = sinon.stub(fs, "ensureDirSync");
+    copySyncStub = sinon.stub(fs, "copySync");
+    chmodSyncStub = sinon.stub(fs, "chmodSync");
+    downloadToTmpStub = sinon.stub(downloadUtils, "downloadToTmp");
+    validateSizeStub = sinon.stub(downloadUtils, "validateSize");
+    validateChecksumStub = sinon.stub(downloadUtils, "validateChecksum");
+  });
+
+  afterEach(() => {
+    sinon.restore();
+    Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true });
+    Object.defineProperty(process, "arch", { value: originalArch, configurable: true });
+
+  });
+
+  it("should return cached binary if valid", async () => {
+    existsSyncStub.returns(true);
+    validateSizeStub.resolves();
+    validateChecksumStub.resolves();
+
+    const result = await getOrDownloadUniversalMaker();
+    expect(existsSyncStub).to.have.been.calledOnce;
+    expect(validateSizeStub).to.have.been.calledOnce;
+    expect(validateChecksumStub).to.have.been.calledOnce;
+    expect(downloadToTmpStub).to.not.have.been.called;
+    expect(result).to.include("universal-maker-linux-x64");
+  });
+
+  it("should redownload if cached binary fails validation", async () => {
+    existsSyncStub.returns(true);
+    // Fail on first call (cache check), succeed on second (downloaded file)
+    validateSizeStub.onFirstCall().rejects(new Error("Invalid size"));
+    validateSizeStub.onSecondCall().resolves();
+    
+    downloadToTmpStub.resolves("/tmp/downloaded_file");
+    validateChecksumStub.resolves(); // For the new file
+
+    const result = await getOrDownloadUniversalMaker();
+    expect(existsSyncStub).to.have.been.calledOnce;
+    expect(validateSizeStub).to.have.been.calledTwice;
+    expect(downloadToTmpStub).to.have.been.calledOnce;
+    expect(copySyncStub).to.have.been.calledOnce;
+    expect(result).to.include("universal-maker-linux-x64");
+  });
+
+  it("should download if binary not in cache", async () => {
+    existsSyncStub.returns(false);
+    downloadToTmpStub.resolves("/tmp/downloaded_file");
+    validateSizeStub.resolves();
+    validateChecksumStub.resolves();
+
+    const result = await getOrDownloadUniversalMaker();
+    expect(existsSyncStub).to.have.been.calledOnce;
+    expect(downloadToTmpStub).to.have.been.calledOnce;
+    expect(copySyncStub).to.have.been.calledOnce;
+    expect(result).to.include("universal-maker-linux-x64");
+  });
+});

src/apphosting/universalMakerDownload.ts

@@ -60,21 +60,22 @@ function getPlatformInfo(): UniversalMakerUpdateDetails {
  * Gets the path to the Universal Maker binary, downloading it if necessary.
  */
 export async function getOrDownloadUniversalMaker(): Promise<string> {
-  if (process.env.UNIVERSAL_MAKER_BINARY) {
-    logger.debug(
-      `[apphosting] Env variable override detected. Using Universal Maker binary at ${process.env.UNIVERSAL_MAKER_BINARY}`,
-    );
-    return process.env.UNIVERSAL_MAKER_BINARY;
-  }
-
   const details = getPlatformInfo();
   const downloadPath = path.join(CACHE_DIR, details.downloadPathRelativeToCacheDir);
 
   const hasBinary = fs.existsSync(downloadPath);
 
   if (hasBinary) {
     logger.debug(`[apphosting] Universal Maker binary found at cache: ${downloadPath}`);
-    return downloadPath;
+    try {
+      await downloadUtils.validateSize(downloadPath, details.expectedSize);
+      await downloadUtils.validateChecksum(downloadPath, details.expectedChecksumSHA256, "sha256");
+      return downloadPath;
+    } catch (err) {
+      logger.warn(
+        `[apphosting] Cached Universal Maker binary failed verification: ${(err as Error).message}. Proceeding to redownload...`,
+      );
+    }
   }
 
   logger.info(

src/downloadUtils.ts

@@ -13,7 +13,7 @@ import { FirebaseError } from "./error";
  * @param remoteUrl URL to download.
  * @param auth Whether to include an access token in the download request. Defaults to false.
  */
-export async function downloadToTmp(remoteUrl: string, auth: boolean = false): Promise<string> {
+export async function downloadToTmp(remoteUrl: string, auth = false): Promise<string> {
   const u = new URL(remoteUrl);
   const c = new Client({ urlPrefix: u.origin, auth });
   const tmpfile = tmp.fileSync();