electron / asar

Simple extensive tar-like archive format with indexing
MIT License
2.55k stars 248 forks source link

fix: relative link error #308

Closed ziqiangai closed 5 months ago

ziqiangai commented 6 months ago

fix the fllowing error in electron-forge framework

> my-electron-vue-app@1.0.0 make
> electron-forge make

✔ Checking your system
⠋ Loading configuration
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
❯ Running package command
  ✔ Preparing to package application
  ❯ Running packaging hooks
    ✔ Running generateAssets hook
✔ Loading configuration
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
✔ Loading configuration
✔ Resolving make targets
✔ Loading configuration
✔ Resolving make targets
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
❯ Running package command
✔ Loading configuration
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
✔ Loading configuration
✔ Resolving make targets
  › Making for the following targets: , 
❯ Running package command
  ✔ Preparing to package application
  ✔ Running packaging hooks
    ✔ Running generateAssets hook
    ✔ Running prePackage hook
      ✔ [plugin-vite] Building vite bundles
  ❯ Packaging application
    ❯ Packaging for arm64 on darwin
      ✔ Copying files
      ✔ Preparing native dependencies [0.5s]
      ✖ Finalizing package
        › /tmp/electron-packager/tmp-oPKOzr/Electron.app/Contents/Resources/app/node_modules/@vue/compiler-core/node_modules/.bin/parser: file "../../../../../../../../tmp/…
  ◼ Running postPackage hook
◼ Running preMake hook
◼ Making distributables
◼ Running postMake hook

An unhandled rejection has occurred inside Forge:
Error: /tmp/electron-packager/tmp-oPKOzr/Electron.app/Contents/Resources/app/node_modules/@vue/compiler-core/node_modules/.bin/parser: file "../../../../../../../../tmp/electron-packager/tmp-oPKOzr/Electron.app/Contents/Resources/app/node_modules/@babel/parser/bin/babel-parser.js" links out of the package
at Filesystem.insertLink (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/asar/lib/filesystem.js:106:13)
    at handleFile (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/asar/lib/asar.js:132:20)
    at next (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/asar/lib/asar.js:148:11)
    at next (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/asar/lib/asar.js:149:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async MacApp.asarApp (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/packager/dist/platform.js:200:9)
    at async MacApp.buildApp (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/packager/dist/platform.js:124:9)
    at async MacApp.initialize (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/packager/dist/platform.js:117:13)
    at async MacApp.create (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/packager/dist/mac.js:321:9)
    at async Promise.all (index 0)
    at async packager (/Users/zzq/Documents/AwesomeCode/my-electron-vue-app/node_modules/@electron/packager/dist/packager.js:237:22)
BlackHole1 commented 6 months ago

This problem is quite complex, and I think I need to provide a detailed description here so that more people can understand the whole context:

In previous versions, asar would ignore the “intermediate” symbolic links. For example, I have three files: A, B, and C, with the relationship being: A -> B -> C. After being processed by asar, B would be discarded, resulting in: A -> C .In most scenarios, this is not a problem. However, if your code hardcodes the path to B, it will result in a "file not found" error.

About a month ago, @JrainLau discovered this issue and submitted a related fix: #301. In his fix, he used fs.readlinkSync + path.dirname to avoid the problem caused by fs.realpathSync (because fs.realpathSync will convert the input path to a path without any symbolic links). To better understand, I will describe the differences between the two in detail:

Input Output
realpathSync /tmp/A /tmp/C
fs.readlinkSync + path.dirname /tmp/A /tmp/B

At that time, we didn’t notice any issues until 3 weeks ago when @viplifes reported a problem. When processing with asar, it incorrectly assumed that the target file was outside the src directory.

At that time, @dsanders11 proposed a fix but didn’t receive a response from @JrainLau until today when @ziqiangai submitted an RP and verified that the modification proposed by @dsanders11 was not feasible. Next, I will explain why the fix code submitted by @JrainLau has this issue.

Before we start, there are two prerequisites to understand:

  1. In macOS, the /var directory actually points to /private/var, as shown in the following figure: 28-28 15 28@2x

  2. Before operating with asar, src is copied to a subdirectory in the /var directory.

  // p = /var/.../Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/.bin/is-docker
  insertLink (p) {
    // symlink = ../is-docker/cli.js
    const symlink = fs.readlinkSync(p)
    // /var/.../Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/.bin
    const parentPath = path.dirname(p)
    // fs.realpathSync(this.src) = /private/var/.../Electron.app/Contents/Resources/app
    // path.join(parentPath, symlink) = /var/.../Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/is-docker/cli.js
    // link = ../../../../../../../../../../../../var/.../Electron.app/Contents/Resources/app/node_modules/is-inside-container/node_modules/is-docker/cli.js
    const link = path.relative(fs.realpathSync(this.src), path.join(parentPath, symlink))
  }

I think everything will become clear after adding comments to the above code. Due to fs.realpathSync(this.src), the path we obtain is /private/var. To solve the issue of missing symbolic links, we cannot use fs.realpathSync for the second parameter, which causes the two prefixes of the paths to be completely inconsistent, ultimately triggering: https://github.com/electron/asar/blob/d50b03f48ce60383a62cf322d7868bc2a183a41e/lib/filesystem.js#L105-L107

As for the fix I mentioned in https://github.com/electron/asar/pull/308#discussion_r1542262308, it involves splitting: Only use fs.realpathSync for the condition check, and use fs.readlinkSync + path.dirname for other places.

ziqiangai commented 6 months ago

LGTM. It would be better if you could add a unit test for this!

It's a bit difficult for me.

BlackHole1 commented 6 months ago

LGTM. It would be better if you could add a unit test for this!

It's a bit difficult for me.

NP 😃. I have pushed the relevant unit tests.

ziqiangai commented 6 months ago

🫡 Congratulations, thank you for letting me learn a lot.

sethyuan commented 5 months ago

The fix can be improved (it's not complete), there are symlinks that resolve to an absolute path too (I ran into this with my Electron project), in which case, it should be ignored (they're usually executable commands like 'python', some dependencies generate these symlinks on building).

The fix I propose is this:

insertLink (p) {
  const symlink = fs.readlinkSync(p)
  // Can't really package these absolute links, they tend to be executable scripts like 'python'.
  if (path.isAbsolute(symlink)) return
  const parentPath = path.dirname(p)
  const link = path.relative(fs.realpathSync(this.src), fs.realpathSync(path.join(parentPath, symlink)))
  if (link.substr(0, 2) === '..') {
    throw new Error(`${p}: file "${link}" links out of the package`)
  }
  const node = this.searchNodeFromPath(p)
  node.link = link
  return link
}
BlackHole1 commented 5 months ago

@sethyuan Thank you for your additional information! I will conduct research in the next few days.

It would be even better if you can provide a reproducible repo, as this can save me a lot of time.😃

sethyuan commented 5 months ago

@BlackHole1 My environment is macOS 14.4.1 (arm64), I have miniconda installed and I'm using an environment created with it. The following steps can reproduce the packaging issue with symlinks:

  1. npm init electron-app@latest my-app -- --template=vite-typescript
  2. cd my-app
  3. yarn add better-sqlite3
  4. yarn package

Basically, it uses electron-forge to build and package the app and better-sqlite3 will build the sqlite3 lib from code leaving a symlink (absolute path) to the 'python' used, which in this case, is the one from miniconda's environment.

Here is a screen capture of the error:

image

BlackHole1 commented 5 months ago

@sethyuan Thanks. If I have enough time today, I will follow up on this issue today.

bstst commented 5 months ago

I'm also encountering a similar issue on OSX 14.2.1 (arm64). The vite-typescript template won't make and of course package.

A fix to make it work in that instance was to add this to forge.config.ts:

  hooks: {
    packageAfterPrune: async (_config, buildPath) => {
      await fs.rm(path.join(
        buildPath,
        'node_modules/scheduler/node_modules/.bin/loose-envify',
      ), {recursive: true, force: true});
      await fs.rm(path.join(
        buildPath,
        'node_modules/react/node_modules/.bin/loose-envify',
      ), {recursive: true, force: true});
      await fs.rm(path.join(
        buildPath,
        'node_modules/react-dom/node_modules/.bin/loose-envify',
      ), {recursive: true, force: true});
   }
  },

I suppose that would also work for node_gyp symlinked bins. But that is not a nice solution.

But in my particular case one of my app dependencies (@istanbuljs) is using esprima, which at make time shoots an error like cannot copy ../../../../../esprima.js to a subdirectory of itself, ../../../../esprima.js, which is related to this https://github.com/electron/forge/issues/3549.

They all seem to be linked to something upstream, something related to symlink resolution.

BlackHole1 commented 5 months ago

I am sorry for the delay. I have been really busy lately.

Thank you @sethyuan and @bstst for the feedback. I have just rethought the repair plan and realized that it does not require a fix as initially discussed by https://github.com/electron/asar/pull/308#discussion_r1542262308.

The main cause of this bug is to address the issue from /var to /private/var. Therefore, I just need to use fs.realpathSync(path.dirname(p)) initially to ensure consistency in the subsequent paths.

@ziqiangai @jrainlau @sethyuan @bstst Can you try the latest repair solution? (It has been verified on my local system already.)

bstst commented 5 months ago

I am sorry for the delay. I have been really busy lately.

Thank you @sethyuan and @bstst for the feedback. I have just rethought the repair plan and realized that it does not require a fix as initially discussed by #308 (comment).

The main cause of this bug is to address the issue from /var to /private/var. Therefore, I just need to use fs.realpathSync(path.dirname(p)) initially to ensure consistency in the subsequent paths.

@ziqiangai @jrainlau @sethyuan @bstst Can you try the latest repair solution? (It has been verified on my local system already.)

The latest solution fixes the issue on my local system 🎉

I do have another somewhat related issue, but am not sure who's to blame:

My project has a dependency on the @smile-cdr/fhirts package. I get this error ("src:" and "dst:" are my debug logs, just as the "catch me" tracer):

src: /Users/martin/Projects/CENSORED/node_modules/@istanbuljs/load-nyc-config/node_modules/js-yaml
dst: /var/folders/bm/ytnswbdd57b5dm28zg7x8ysc0000gn/T/electron-packager/tmp-8AxS5d/Electron.app/Contents/Resources/app/node_modules/@istanbuljs/load-nyc-config/node_modules/js-yaml

Trace: catch me
    at /Users/martin/Projects/CENSORED/node_modules/fs-extra/lib/copy/copy.js:213:19
    at FSReqCallback.oncomplete (node:fs:200:23)

An unhandled rejection has occurred inside Forge:
Error: Cannot copy '../../../../../../esprima/bin/esvalidate.js' to a subdirectory of itself, '../../../../../../esprima/bin/esvalidate.js'.
at /Users/martin/Projects/CENSORED/node_modules/fs-extra/lib/copy/copy.js:214:21
    at FSReqCallback.oncomplete (node:fs:200:23)

That comes from them having a dependency on "nyc" in their "dependencies" section. If I move it to "devDependencies" it works fine. Which makes sense since it's all tests.

But the actual error comes, I believe, from the fact that they're all symlinks and it tries to copy a symlink into a symlink or something like that (I didn't spend much time on that). Is the "fs-extra" package here to blame (https://github.com/jprichardson/node-fs-extra/issues/1019)?

sethyuan commented 5 months ago

I am sorry for the delay. I have been really busy lately.

Thank you @sethyuan and @bstst for the feedback. I have just rethought the repair plan and realized that it does not require a fix as initially discussed by #308 (comment).

The main cause of this bug is to address the issue from /var to /private/var. Therefore, I just need to use fs.realpathSync(path.dirname(p)) initially to ensure consistency in the subsequent paths.

@ziqiangai @jrainlau @sethyuan @bstst Can you try the latest repair solution? (It has been verified on my local system already.)

Yes, it does work for me, thanks!

BlackHole1 commented 5 months ago

I do have another somewhat related issue, but am not sure who's to blame:

@bstst Can you try again to use the @electron/asar@3.2.8 version? If the problem still exists, then it is unrelated to #301 and #308, you should submit a new issue.

BTW, thank you @bstst @sethyuan for the feedback :)

bstst commented 5 months ago

I do have another somewhat related issue, but am not sure who's to blame:

@bstst Can you try again to use the @electron/asar@3.2.8 version? If the problem still exists, then it is unrelated to #301 and #308, you should submit a new issue.

BTW, thank you @bstst @sethyuan for the feedback :)

Does not seem like this package is related to the other issue that I'm getting ("fs-extra".copy is being called directly from the @electron-forge/vite-plugin, so it looks like the issue is indeed on the "fs-extra" side, will continue with them).

So it looks like this PR does indeed fix the "links out of the package" issue. Thanks for the effort everyone!

continuous-auth[bot] commented 5 months ago

:tada: This PR is included in version 3.2.10 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

twann503 commented 2 months ago

I have this same issue even after verifying that my asar package is at version 3.2.10. any ideas on how to fix this? seems to happen when I add this package @atlaskit/editor-common

the error

An unhandled rejection has occurred inside Forge: Error: Cannot copy '../../../../../../markdown-it/bin/markdown-it.mjs' to a subdirectory of itself, '../../../../../../markdown-it/bin/markdown-it.mjs'. at /Users/alex.pham/Documents/Git/vitelasttry/my-new-app-2/node_modules/fs-extra/lib/copy/copy.js:213:21 at FSReqCallback.oncomplete (node:fs:193:23) error Command failed with exit code 1.

erickzhao commented 2 months ago

@twann503 I don't think this is the same error. The error for this one is links out of the package and yours says Cannot copy to a subdirectory of itself.

twann503 commented 2 months ago

very similar to @bstst error though with being unable to copy because the symlink issue.