Repository: netlify/zip-it-and-ship-it
Pull request #176 (2020-08-04 18:07:25)

Ensure Zip archives contents are deterministic
The order in which files are added to a Zip archive does not change how they are extracted. However, this changes the archive's checksum.

Producing non-deterministic Zip archives prevents caching them based on their checksum. This is currently a problem in our API (https://github.com/netlify/ansible/issues/4282).

This PR ensures files are added in a deterministic order to the Zip archive by:
  - Sorting them by file path
  - Ensuring any parallelism does not make the archive non-deterministic

This also adds a regression test.


Patch:
diff --git a/package-lock.json b/package-lock.json
index 76acfad..c46bbe4 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -4770,10 +4770,13 @@
       "dev": true
     },
     "get-stream": {
-      "version": "3.0.0",
-      "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-3.0.0.tgz",
-      "integrity": "sha1-jpQ9E1jcN1VQVOy+LtsFqhdO3hQ=",
-      "dev": true
+      "version": "5.1.0",
+      "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-5.1.0.tgz",
+      "integrity": "sha512-EXr1FOzrzTfGeL0gQdeFEvOMm2mzMOglyiOXSTpPC+iAjAKftbr3jpCMWynogwYnM+eSj9sHGc6wjIcDvYiygw==",
+      "dev": true,
+      "requires": {
+        "pump": "^3.0.0"
+      }
     },
     "get-value": {
       "version": "2.0.6",
@@ -4978,6 +4981,14 @@
             "timed-out": "^4.0.0",
             "unzip-response": "^2.0.1",
             "url-parse-lax": "^1.0.0"
+          },
+          "dependencies": {
+            "get-stream": {
+              "version": "3.0.0",
+              "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-3.0.0.tgz",
+              "integrity": "sha1-jpQ9E1jcN1VQVOy+LtsFqhdO3hQ=",
+              "dev": true
+            }
           }
         },
         "inquirer": {
@@ -9141,6 +9152,14 @@
             "p-finally": "^1.0.0",
             "signal-exit": "^3.0.0",
             "strip-eof": "^1.0.0"
+          },
+          "dependencies": {
+            "get-stream": {
+              "version": "3.0.0",
+              "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-3.0.0.tgz",
+              "integrity": "sha1-jpQ9E1jcN1VQVOy+LtsFqhdO3hQ=",
+              "dev": true
+            }
           }
         },
         "is-stream": {
diff --git a/package.json b/package.json
index eb08a80..5f3ffef 100644
--- a/package.json
+++ b/package.json
@@ -85,6 +85,7 @@
     "eslint-plugin-node": "^11.1.0",
     "eslint-plugin-prettier": "^3.1.4",
     "execa": "^3.4.0",
+    "get-stream": "^5.1.0",
     "gh-release": "^3.5.0",
     "husky": "^4.2.5",
     "npm-run-all": "^4.1.5",
diff --git a/src/dependencies.js b/src/dependencies.js
index 60067b9..7116c25 100644
--- a/src/dependencies.js
+++ b/src/dependencies.js
@@ -18,7 +18,9 @@ const listNodeFiles = async function(srcPath, filename, mainFile, srcDir, stat)
   const [treeFiles, depFiles] = await Promise.all([getTreeFiles(srcPath, stat), getDependencies(mainFile, srcDir)])
   const files = [...treeFiles, ...depFiles].map(normalize)
   const uniqueFiles = [...new Set(files)]
-  const filteredFiles = uniqueFiles.filter(isNotJunk)
+
+  // We sort so that the archive's checksum is deterministic.
+  const filteredFiles = uniqueFiles.filter(isNotJunk).sort()
   return filteredFiles
 }
 
diff --git a/src/helpers/sha.js b/src/helpers/sha.js
new file mode 100644
index 0000000..b1309fd
--- /dev/null
+++ b/src/helpers/sha.js
@@ -0,0 +1,16 @@
+const { createHash } = require('crypto')
+const { createReadStream } = require('fs')
+
+const getStream = require('get-stream')
+
+// Retrieve the SHA1 checksum of a file.
+// Does it in streaming mode, for best performance.
+const computeSha1 = async function(filePath) {
+  const fileStream = createReadStream(filePath)
+  const hashStream = createHash('sha1')
+  hashStream.setEncoding('hex')
+  const sha1Checksum = await getStream(fileStream.pipe(hashStream))
+  return sha1Checksum
+}
+
+module.exports = { computeSha1 }
diff --git a/src/node.js b/src/node.js
index 7322721..b89acdf 100644
--- a/src/node.js
+++ b/src/node.js
@@ -18,7 +18,13 @@ const zipNodeJs = async function(srcFiles, destPath, filename, mainFile) {
 
   addEntryFile(commonPrefix, archive, filename, mainFile)
 
-  await Promise.all(srcFiles.map(file => zipJsFile(file, commonPrefix, archive)))
+  const srcFilesInfos = await Promise.all(srcFiles.map(addStat))
+
+  // We ensure this is not async, so that the archive's checksum is
+  // deterministic. Otherwise it depends on the order the files were added.
+  srcFilesInfos.forEach(({ srcFile, stat }) => {
+    zipJsFile({ srcFile, commonPrefix, archive, stat })
+  })
 
   await endZip(archive, output)
 }
@@ -31,10 +37,14 @@ const addEntryFile = function(commonPrefix, archive, filename, mainFile) {
   addZipContent(archive, content, entryFilename)
 }
 
-const zipJsFile = async function(file, commonPrefix, archive) {
-  const filename = normalizeFilePath(file, commonPrefix)
-  const stat = await pStat(file)
-  addZipFile(archive, file, filename, stat)
+const addStat = async function(srcFile) {
+  const stat = await pStat(srcFile)
+  return { srcFile, stat }
+}
+
+const zipJsFile = function({ srcFile, commonPrefix, archive, stat }) {
+  const filename = normalizeFilePath(srcFile, commonPrefix)
+  addZipFile(archive, srcFile, filename, stat)
 }
 
 // `adm-zip` and `require()` expect Unix paths.


Test patch:
diff --git a/src/main.test.js b/src/main.test.js
index b377865..f419f09 100644
--- a/src/main.test.js
+++ b/src/main.test.js
@@ -14,6 +14,7 @@ const promisify = require('util.promisify')
 const { zipFunction, listFunctions, listFunctionsFiles } = require('..')
 
 const { zipNode, zipFixture, unzipFiles, zipCheckFunctions, FIXTURES_DIR } = require('./helpers/main.js')
+const { computeSha1 } = require('./helpers/sha.js')
 
 const pReadFile = promisify(readFile)
 const pChmod = promisify(chmod)
@@ -178,6 +179,19 @@ test('Works with many function files', async t => {
   await zipNode(t, 'many-functions', 6)
 })
 
+test('Produces deterministic checksums', async t => {
+  const [checksumOne, checksumTwo] = await Promise.all([getZipChecksum(t), getZipChecksum(t)])
+  t.is(checksumOne, checksumTwo)
+})
+
+const getZipChecksum = async function(t) {
+  const {
+    files: [{ path }]
+  } = await zipFixture(t, 'many-dependencies')
+  const sha1sum = computeSha1(path)
+  return sha1sum
+}
+
 test('Throws when the source folder does not exist', async t => {
   await t.throwsAsync(zipNode(t, 'does-not-exist'), /Functions folder does not exist/)
 })