fix: Respect unpack minimatch for symlinks within previously unpacked directories (#341)

* fix: Respecting unpack configuration when considering symlinks within previously unpacked directories. This directly fixes unpacking static `.framework` modules on Mac, as otherwise codesigning will fail due to symlink files/directories not being reflected in the app.asar.unpacked directory.

Added unit test with Hello.framework, generated from tutorial https://jano.dev/apple/mach-o/2024/06/28/Hello-Static-Framework.html
Fixes: electron-userland/electron-builder#8655

* adding unit test by programmatically create symlinks during test case (same approach as has been taken for filesystem UT already)

* cleanup changes post-merging `main`

Author: mmaietta

src/asar.ts

@@ -80,7 +80,8 @@ export async function createPackageFromFiles(
   });
 
   const filesystem = new Filesystem(src);
-  const files: { filename: string; unpack: boolean }[] = [];
+  const files: disk.BasicFilesArray = [];
+  const links: disk.BasicFilesArray = [];
   const unpackDirs: string[] = [];
 
   let filenamesSorted: string[] = [];
@@ -140,37 +141,51 @@ export async function createPackageFromFiles(
     }
     const file = metadata[filename];
 
+    const shouldUnpackPath = function (
+      relativePath: string,
+      unpack: string | undefined,
+      unpackDir: string | undefined,
+    ) {
+      let shouldUnpack = false;
+      if (unpack) {
+        shouldUnpack = minimatch(filename, unpack, { matchBase: true });
+      }
+      if (!shouldUnpack && unpackDir) {
+        shouldUnpack = isUnpackedDir(relativePath, unpackDir, unpackDirs);
+      }
+      return shouldUnpack;
+    };
+
     let shouldUnpack: boolean;
     switch (file.type) {
       case 'directory':
-        if (options.unpackDir) {
-          shouldUnpack = isUnpackedDir(path.relative(src, filename), options.unpackDir, unpackDirs);
-        } else {
-          shouldUnpack = false;
-        }
+        shouldUnpack = shouldUnpackPath(path.relative(src, filename), undefined, options.unpackDir);
         filesystem.insertDirectory(filename, shouldUnpack);
         break;
       case 'file':
-        shouldUnpack = false;
-        if (options.unpack) {
-          shouldUnpack = minimatch(filename, options.unpack, { matchBase: true });
-        }
-        if (!shouldUnpack && options.unpackDir) {
-          const dirName = path.relative(src, path.dirname(filename));
-          shouldUnpack = isUnpackedDir(dirName, options.unpackDir, unpackDirs);
-        }
-        files.push({ filename: filename, unpack: shouldUnpack });
+        shouldUnpack = shouldUnpackPath(
+          path.relative(src, path.dirname(filename)),
+          options.unpack,
+          options.unpackDir,
+        );
+        files.push({ filename, unpack: shouldUnpack });
         return filesystem.insertFile(filename, shouldUnpack, file, options);
       case 'link':
-        filesystem.insertLink(filename);
+        shouldUnpack = shouldUnpackPath(
+          path.relative(src, filename),
+          options.unpack,
+          options.unpackDir,
+        );
+        links.push({ filename, unpack: shouldUnpack });
+        filesystem.insertLink(filename, shouldUnpack);
         break;
     }
     return Promise.resolve();
   };
 
   const insertsDone = async function () {
     await fs.mkdirp(path.dirname(dest));
-    return disk.writeFilesystem(dest, filesystem, files, metadata);
+    return disk.writeFilesystem(dest, filesystem, { files, links }, metadata);
   };
 
   const names = filenamesSorted.slice();

src/disk.ts

@@ -37,14 +37,17 @@ export type InputMetadata = {
 
 export type BasicFilesArray = { filename: string; unpack: boolean }[];
 
+export type FilesystemFilesAndLinks = { files: BasicFilesArray; links: BasicFilesArray };
+
 const writeFileListToStream = async function (
   dest: string,
   filesystem: Filesystem,
   out: NodeJS.WritableStream,
-  fileList: BasicFilesArray,
+  lists: FilesystemFilesAndLinks,
   metadata: InputMetadata,
 ) {
-  for (const file of fileList) {
+  const { files, links } = lists;
+  for (const file of files) {
     if (file.unpack) {
       // the file should not be packed into archive
       const filename = path.relative(filesystem.getRootPath(), file.filename);
@@ -53,13 +56,30 @@ const writeFileListToStream = async function (
       await streamTransformedFile(file.filename, out, metadata[file.filename].transformed);
     }
   }
+  const unpackedSymlinks = links.filter((f) => f.unpack);
+  for (const file of unpackedSymlinks) {
+    // the symlink needs to be recreated outside in .unpacked
+    const filename = path.relative(filesystem.getRootPath(), file.filename);
+    const link = await fs.readlink(file.filename);
+    // if symlink is within subdirectories, then we need to recreate dir structure
+    await fs.mkdirp(path.join(`${dest}.unpacked`, path.dirname(filename)));
+    // create symlink within unpacked dir
+    await fs.symlink(link, path.join(`${dest}.unpacked`, filename)).catch(async (error) => {
+      if (error.code === 'EPERM' && error.syscall === 'symlink') {
+        throw new Error(
+          'Could not create symlinks for unpacked assets. On Windows, consider activating Developer Mode to allow non-admin users to create symlinks by following the instructions at https://docs.microsoft.com/en-us/windows/apps/get-started/enable-your-device-for-development.',
+        );
+      }
+      throw error;
+    });
+  }
   return out.end();
 };
 
 export async function writeFilesystem(
   dest: string,
   filesystem: Filesystem,
-  fileList: BasicFilesArray,
+  lists: FilesystemFilesAndLinks,
   metadata: InputMetadata,
 ) {
   const headerPickle = Pickle.createEmpty();
@@ -76,7 +96,7 @@ export async function writeFilesystem(
     out.write(sizeBuf);
     return out.write(headerBuf, () => resolve());
   });
-  return writeFileListToStream(dest, filesystem, out, fileList, metadata);
+  return writeFileListToStream(dest, filesystem, out, lists, metadata);
 }
 
 export interface FileRecord extends FilesystemFileEntry {

src/filesystem.ts

@@ -156,7 +156,7 @@ export class Filesystem {
     this.offset += BigInt(size);
   }
 
-  insertLink(p: string) {
+  insertLink(p: string, shouldUnpack: boolean) {
     const symlink = fs.readlinkSync(p);
     // /var => /private/var
     const parentPath = fs.realpathSync(path.dirname(p));
@@ -165,6 +165,10 @@ export class Filesystem {
       throw new Error(`${p}: file "${link}" links out of the package`);
     }
     const node = this.searchNodeFromPath(p) as FilesystemLinkEntry;
+    const dirNode = this.searchNodeFromPath(path.dirname(p)) as FilesystemDirectoryEntry;
+    if (shouldUnpack || dirNode.unpacked) {
+      node.unpacked = true;
+    }
     node.link = link;
     return link;
   }

src/wrapped-fs.ts

@@ -1,6 +1,14 @@
 const fs = 'electron' in process.versions ? require('original-fs') : require('fs');
 
-const promisifiedMethods = ['lstat', 'mkdtemp', 'readFile', 'stat', 'writeFile'];
+const promisifiedMethods = [
+  'lstat',
+  'mkdtemp',
+  'readFile',
+  'stat',
+  'writeFile',
+  'symlink',
+  'readlink',
+];
 
 type AsarFS = typeof import('fs') & {
   mkdirp(dir: string): Promise<void>;
@@ -10,6 +18,8 @@ type AsarFS = typeof import('fs') & {
   readFile: (typeof import('fs'))['promises']['readFile'];
   stat: (typeof import('fs'))['promises']['stat'];
   writeFile: (typeof import('fs'))['promises']['writeFile'];
+  symlink: (typeof import('fs'))['promises']['symlink'];
+  readlink: (typeof import('fs'))['promises']['readlink'];
 };
 
 const promisified = {} as AsarFS;

test/cli-spec.js

@@ -11,6 +11,7 @@ const rimraf = require('rimraf');
 const compDirs = require('./util/compareDirectories');
 const compFileLists = require('./util/compareFileLists');
 const compFiles = require('./util/compareFiles');
+const createSymlinkApp = require('./util/createSymlinkApp');
 
 const exec = promisify(childProcess.exec);
 
@@ -188,4 +189,22 @@ describe('command line interface', function () {
       ) === false,
     );
   });
+  it('should unpack static framework with all underlying symlinks unpacked', async () => {
+    const { tmpPath } = createSymlinkApp('app');
+    await execAsar(
+      `p ${tmpPath} tmp/packthis-with-symlink.asar --unpack *.txt --unpack-dir var --exclude-hidden`,
+    );
+
+    assert.ok(fs.existsSync('tmp/packthis-with-symlink.asar.unpacked/private/var/file.txt'));
+    assert.ok(fs.existsSync('tmp/packthis-with-symlink.asar.unpacked/private/var/app/file.txt'));
+    assert.strictEqual(
+      fs.readlinkSync('tmp/packthis-with-symlink.asar.unpacked/private/var/app/file.txt'),
+      path.join('..', 'file.txt'),
+    );
+    assert.strictEqual(
+      fs.readlinkSync('tmp/packthis-with-symlink.asar.unpacked/var'),
+      path.join('private', 'var'),
+    );
+    assert.ok(fs.existsSync('tmp/packthis-with-symlink.asar.unpacked/var/file.txt'));
+  });
 });

test/filesystem-spec.js

@@ -4,6 +4,7 @@ const assert = require('assert');
 const fs = require('../lib/wrapped-fs').default;
 const path = require('path');
 const rimraf = require('rimraf');
+const createSymlinkedApp = require('./util/createSymlinkApp');
 
 const Filesystem = require('../lib/filesystem').Filesystem;
 
@@ -13,28 +14,7 @@ describe('filesystem', function () {
   });
 
   it('should does not throw an error when the src path includes a symbol link', async () => {
-    /**
-     * Directory structure:
-     * tmp
-     * ├── private
-     * │   └── var
-     * │       ├── app
-     * │       │   └── file.txt -> ../file.txt
-     * │       └── file.txt
-     * └── var -> private/var
-     */
-    const tmpPath = path.join(__dirname, '..', 'tmp');
-    const privateVarPath = path.join(tmpPath, 'private', 'var');
-    const varPath = path.join(tmpPath, 'var');
-    fs.mkdirSync(privateVarPath, { recursive: true });
-    fs.symlinkSync(path.relative(tmpPath, privateVarPath), varPath);
-
-    const originFilePath = path.join(varPath, 'file.txt');
-    fs.writeFileSync(originFilePath, 'hello world');
-    const appPath = path.join(varPath, 'app');
-    fs.mkdirpSync(appPath);
-    fs.symlinkSync('../file.txt', path.join(appPath, 'file.txt'));
-
+    const { appPath, varPath } = createSymlinkedApp('filesystem');
     const filesystem = new Filesystem(varPath);
     assert.doesNotThrow(() => {
       filesystem.insertLink(path.join(appPath, 'file.txt'));

test/util/createSymlinkApp.js

@@ -0,0 +1,30 @@
+const path = require('path');
+const fs = require('../../lib/wrapped-fs').default;
+const rimraf = require('rimraf');
+/**
+ * Directory structure:
+ * tmp
+ * ├── private
+ * │   └── var
+ * │       ├── app
+ * │       │   └── file.txt -> ../file.txt
+ * │       └── file.txt
+ * └── var -> private/var
+ */
+module.exports = (testName) => {
+  const tmpPath = path.join(__dirname, '../..', 'tmp', testName || 'app');
+  const privateVarPath = path.join(tmpPath, 'private', 'var');
+  const varPath = path.join(tmpPath, 'var');
+
+  rimraf.sync(tmpPath, fs);
+
+  fs.mkdirSync(privateVarPath, { recursive: true });
+  fs.symlinkSync(path.relative(tmpPath, privateVarPath), varPath);
+
+  const originFilePath = path.join(varPath, 'file.txt');
+  fs.writeFileSync(originFilePath, 'hello world');
+  const appPath = path.join(varPath, 'app');
+  fs.mkdirpSync(appPath);
+  fs.symlinkSync('../file.txt', path.join(appPath, 'file.txt'));
+  return { appPath, tmpPath, varPath };
+};