fix: preserve symlinks from included files in the resulting zip archive (#1701)

* fix: preserve symlinks from included files in the resulting zip archive

* chore: add the fix

* chore: fix windows

* chore: pr feedback from eduardo

* chore: fix windows

* chore: skip on windows

Author: Lukas Holzer

package-lock.json

@@ -102,10 +102,10 @@
     "@types/unixify": "1.0.2",
     "@types/yargs": "17.0.32",
     "@vitest/coverage-v8": "0.34.6",
-    "adm-zip": "0.5.10",
     "browserslist": "4.22.2",
     "cardinal": "2.1.1",
     "cpy": "9.0.1",
+    "decompress": "^4.2.1",
     "deepmerge": "4.3.1",
     "get-stream": "8.0.1",
     "husky": "8.0.3",

package.json

@@ -53,9 +53,13 @@ export const getPathsOfIncludedFiles = async (
     cwd: basePath,
     dot: true,
     ignore: excludePatterns,
-    followSymbolicLinks: true,
-    throwErrorOnBrokenSymbolicLink: true,
+    onlyFiles: false,
+    // get directories as well to get symlinked directories,
+    // to filter the regular non symlinked directories out mark them with a slash at the end to filter them out.
+    markDirectories: true,
+    followSymbolicLinks: false,
   })
 
-  return { excludePatterns, paths: pathGroups.map(normalize) }
+  // now filter the non symlinked directories out that got marked with a trailing slash
+  return { excludePatterns, paths: pathGroups.filter((path) => !path.endsWith('/')).map(normalize) }
 }

src/runtimes/node/utils/included_files.ts

@@ -0,0 +1,6 @@
+export default async () =>
+  new Response('<h1>Hello world</h1>', {
+    headers: {
+      'content-type': 'text/html',
+    },
+  })

tests/fixtures-esm/symlinked-included-files/function.mjs

@@ -1,11 +1,12 @@
-import { mkdir, readFile, chmod, symlink, writeFile, rm } from 'fs/promises'
+import { mkdir, readFile, rm, symlink, writeFile } from 'fs/promises'
 import { dirname, isAbsolute, join, resolve } from 'path'
-import { arch, platform, version as nodeVersion } from 'process'
+import { arch, version as nodeVersion, platform } from 'process'
 
-import AdmZip from 'adm-zip'
 import cpy from 'cpy'
+import decompress from 'decompress'
 import merge from 'deepmerge'
 import { execa, execaNode } from 'execa'
+import glob from 'fast-glob'
 import isCI from 'is-ci'
 import { pathExists } from 'path-exists'
 import semver from 'semver'
@@ -22,15 +23,15 @@ import { shellUtils } from '../src/utils/shell.js'
 import type { ZipFunctionsOptions } from '../src/zip.js'
 
 import {
+  BINARY_PATH,
+  FIXTURES_DIR,
+  getBundlerNameFromOptions,
   getRequires,
-  zipNode,
-  zipFixture,
+  importFunctionFile,
   unzipFiles,
   zipCheckFunctions,
-  FIXTURES_DIR,
-  BINARY_PATH,
-  importFunctionFile,
-  getBundlerNameFromOptions,
+  zipFixture,
+  zipNode,
 } from './helpers/main.js'
 import { computeSha1 } from './helpers/sha.js'
 import { allBundleConfigs, testMany } from './helpers/test_many.js'
@@ -40,8 +41,6 @@ import 'source-map-support/register'
 
 vi.mock('../src/utils/shell.js', () => ({ shellUtils: { runCommand: vi.fn() } }))
 
-const EXECUTABLE_PERMISSION = 0o755
-
 const getZipChecksum = async function (opts: ZipFunctionsOptions) {
   const {
     files: [{ path }],
@@ -1754,16 +1753,6 @@ describe('zip-it-and-ship-it', () => {
     const unzippedFile = `${unzippedFunctions[0].unzipPath}/bootstrap`
     await expect(unzippedFile).toPathExist()
 
-    // The library we use for unzipping does not keep executable permissions.
-    // https://github.com/cthackers/adm-zip/issues/86
-    // However `chmod()` is not cross-platform
-    if (platform === 'linux') {
-      await chmod(unzippedFile, EXECUTABLE_PERMISSION)
-
-      const { stdout } = await execa(unzippedFile)
-      expect(stdout).toBe('Hello, world!')
-    }
-
     const tcFile = `${unzippedFunctions[0].unzipPath}/netlify-toolchain`
     await expect(tcFile).toPathExist()
     const tc = await readFile(tcFile, 'utf8')
@@ -2853,7 +2842,7 @@ describe('zip-it-and-ship-it', () => {
   })
 
   testMany('only includes files once in a zip', [...allBundleConfigs], async (options) => {
-    const { files } = await zipFixture('local-require', {
+    const { files, tmpDir } = await zipFixture('local-require', {
       length: 1,
       opts: merge(options, {
         basePath: join(FIXTURES_DIR, 'local-require'),
@@ -2866,9 +2855,11 @@ describe('zip-it-and-ship-it', () => {
       }),
     })
 
-    const zip = new AdmZip(files[0].path)
+    const unzipPath = join(tmpDir, 'unzipped')
+
+    await decompress(files[0].path, unzipPath)
 
-    const fileNames: string[] = zip.getEntries().map((entry) => entry.entryName)
+    const fileNames: string[] = await glob('**', { dot: true, cwd: unzipPath })
     const duplicates = fileNames.filter((item, index) => fileNames.indexOf(item) !== index)
     expect(duplicates).toHaveLength(0)
   })

tests/fixtures-esm/symlinked-included-files/node_modules/.pnpm/crazy-dep/package.json

@@ -0,0 +1,67 @@
+import { readdir } from 'fs/promises'
+import { platform } from 'os'
+import { join } from 'path'
+
+import decompress from 'decompress'
+import { dir as getTmpDir } from 'tmp-promise'
+import { expect, test } from 'vitest'
+
+import { ARCHIVE_FORMAT, zipFunction } from '../src/main.js'
+
+import { FIXTURES_ESM_DIR } from './helpers/main.js'
+
+/** Small helper function, reading a directory recursively and returning a record with the files and if it is a symlink or not */
+const readDirWithType = async (dir: string, readFiles?: Record<string, boolean>, parent = '') => {
+  const files: Record<string, boolean> = readFiles || {}
+  const dirents = await readdir(dir, { withFileTypes: true })
+
+  for (const dirent of dirents) {
+    if (dirent.isDirectory()) {
+      await readDirWithType(join(dir, dirent.name), files, dirent.name)
+    } else {
+      files[join(parent, dirent.name)] = dirent.isSymbolicLink()
+    }
+  }
+
+  return files
+}
+
+test.skipIf(platform() === 'win32')('Symlinked directories from `includedFiles` are preserved', async () => {
+  const { path: tmpDir } = await getTmpDir({ prefix: 'zip-it-test' })
+  const basePath = join(FIXTURES_ESM_DIR, 'symlinked-included-files')
+  const mainFile = join(basePath, 'function.mjs')
+
+  // assert on the source files
+  expect(await readDirWithType(basePath)).toEqual({
+    'function.mjs': false,
+    [join('crazy-dep/package.json')]: false,
+    [join('node_modules/crazy-dep')]: true,
+  })
+
+  const result = await zipFunction(mainFile, tmpDir, {
+    archiveFormat: ARCHIVE_FORMAT.ZIP,
+    basePath,
+    config: {
+      '*': {
+        includedFiles: ['**'],
+      },
+    },
+    featureFlags: {},
+    repositoryRoot: basePath,
+    systemLog: console.log,
+    debug: true,
+    internalSrcFolder: undefined,
+  })
+
+  const unzippedPath = join(tmpDir, 'extracted')
+  await decompress(result!.path, unzippedPath)
+
+  // expect that the symlink for `node_modules/crazy-dep` is preserved
+  expect(await readDirWithType(unzippedPath)).toEqual({
+    '___netlify-bootstrap.mjs': false,
+    '___netlify-entry-point.mjs': false,
+    'function.mjs': false,
+    [join('crazy-dep/package.json')]: false,
+    [join('node_modules/crazy-dep')]: true,
+  })
+})