isUnpackedDir unintentionally unpacks other directories with same dir name prefix

[mmaietta]

Hello all

I'm working on migrating electron-builder to leverage electron/asar instead of its in-house solution. In one of our unit tests I have the following structure:

/project# tree "/tmp/et-db411cd21f3bbd882f3a5a30c58db6ff/t-jp88Jv/asar-app-1"
/tmp/et-db411cd21f3bbd882f3a5a30c58db6ff/t-jp88Jv/asar-app-1
├── app
│   ├── package.json
│   │   └── readme.md
│   └── readme.md
├── index.html
├── index.js
├── node_modules
│   ├── @electron-builder
│   │   ├── test-smart-unpack
│   │   │   ├── foo.dll
│   │   │   └── package.json
│   │   └── test-smart-unpack-empty
│   │       └── package.json

test-smart-unpack is auto-detected by electron-builder to unpack due to the underlying .dll file, and in turn, that minimatch is passed to electron/asar to unpack that dir with the pattern below

pattern = "{node_modules/@electron-builder/test-smart-unpack,node_modules/edge-cs}"

asar/src/asar.ts

Lines 22 to 31 in 464e834

	function isUnpackedDir(dirPath: string, pattern: string, unpackDirs: string[]) {
	if (dirPath.startsWith(pattern) || minimatch(dirPath, pattern)) {
	if (!unpackDirs.includes(dirPath)) {
	unpackDirs.push(dirPath);
	}
	return true;
	} else {
	return unpackDirs.some((unpackDir) => dirPath.startsWith(unpackDir));
	}
	}

The issue I'm running into is that test-smart-unpack-empty is not supposed to be getting unpacked as it fails the minimatch, but the else logic in isUnpackedDir is causing it to do a startsWith match on the folder path and then returns true.

What I think the else logic is attempting to do is to determine if the dirPath is within the tree hierarchy of a previously-unpacked dir. Is that right?

asar/src/asar.ts

Line 29 in 464e834

return unpackDirs.some((unpackDir) => dirPath.startsWith(unpackDir));

If so, I think an additional check is needed such that we check that the relative path is within the dir path hierarchy, and not just a startsWith check.
Maybe something like: dirPath.startsWith(unpackDir) && !path.relative(dirPath, unpackDir).startsWith("../")?

Wanted to post an issue here first to touch base with you all, in case I'm doing something wrong, before diving in deeper to see if I can PR something

[Akarinx]

maybe it's the same problem like #303, and the same fix method

[mmaietta]

@Akarinx I gave the fix in #333 a test for #303 and it does not fix your issue.

I might be able to take a deeper look though when I have some time

[Akarinx]

@mmaietta
I mean this line has the same issue

asar/src/crawlfs.ts

Line 51 in b773063

return !filename.startsWith(link);

and it maybe can be fixed in the same method

[mmaietta]

Currently investigating around with that already but it doesn't seem to work the same way

Found a solution. Opening a separate PR

[continuous-auth]

🎉 This issue has been resolved in version 3.2.17 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀