asar icon indicating copy to clipboard operation
asar copied to clipboard

`isUnpackedDir` unintentionally unpacks other directories with same dir name prefix

Open mmaietta opened this issue 1 year ago • 0 comments

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}"

https://github.com/electron/asar/blob/464e83436967438c74d8ac184b088fc780706b2d/src/asar.ts#L22-L31

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. Image

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? https://github.com/electron/asar/blob/464e83436967438c74d8ac184b088fc780706b2d/src/asar.ts#L29 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

mmaietta avatar Oct 07 '24 23:10 mmaietta