Skip to content

fix(unix): don't rewrite app.asar.unpacked paths a second time - #924

Open
arthur791004 wants to merge 1 commit into
microsoft:mainfrom
arthur791004:fix-asar-unpacked-double-replace
Open

fix(unix): don't rewrite app.asar.unpacked paths a second time#924
arthur791004 wants to merge 1 commit into
microsoft:mainfrom
arthur791004:fix-asar-unpacked-double-replace

Conversation

@arthur791004

Copy link
Copy Markdown

Summary

Fixes #923.

src/unixTerminal.ts#L19-L20 does:

helperPath = helperPath.replace('app.asar', 'app.asar.unpacked');
helperPath = helperPath.replace('node_modules.asar', 'node_modules.asar.unpacked');

String.prototype.replace with a string argument matches the first occurrence. When the resolved `helperPath` already contains `app.asar.unpacked/…` (e.g. a caller of node-pty that itself lives inside `app.asar.unpacked`, bypassing Electron's asar shim — see #923 for the concrete repro), the substring `app.asar` matches the prefix of `app.asar.unpacked` and we produce a bogus `…/app.asar.unpacked.unpacked/…` path. `posix_spawn` then fails with `ENOENT` and node-pty surfaces the misleading `posix_spawnp failed.`.

This PR guards each rewrite so it only fires when the unpacked variant isn't already present.

Test plan

  • Existing unix-terminal tests still pass (`npm test` → 17/17).
  • Manual repro on macOS arm64 from the linked issue's setup: before this patch `pty.spawn` throws `posix_spawnp failed.`; after this patch it spawns normally.

helperPath.replace('app.asar', 'app.asar.unpacked') matches the first
occurrence. When helperPath already contains 'app.asar.unpacked/...'
(e.g. a caller of node-pty that itself lives inside app.asar.unpacked,
bypassing Electron's asar shim), the substring 'app.asar' matches the
prefix of 'app.asar.unpacked' and we produce 'app.asar.unpacked.unpacked/...'
— a path that doesn't exist on disk. posix_spawn then fails with ENOENT
and node-pty throws the misleading "posix_spawnp failed.".

Skip each rewrite when the unpacked variant is already present.

Fixes microsoft#923.
@feiiiiii5

Copy link
Copy Markdown

Hi — we hit this exact bug in a packaged Electron app on macOS 26 and verified your fix end-to-end. Sharing the evidence and a suggestion:

Reproduction (real runtime): loading node-pty 1.1.0 from an asar layout where it lives under app.asar.unpacked (the standard layout for native modules), every PTY spawn fails with posix_spawnp failed. even for /usr/bin/true. Captured helperPath passed to the native fork: .../app.asar.unpacked.unpacked/node_modules/node-pty/build/Release/spawn-helper. Plain child_process.spawn works, and a byte-level replication of the native spawn sequence with the correct helper path succeeds — confirming the failure is purely the rewritten helper path.

Your patch works: with the guarded rewrite applied, a real /bin/bash PTY spawn succeeds in the same runtime.

Suggestion — this needs tests: the inline rewrite is untestable as written. I've opened #953 which extracts resolveSpawnHelperPath() into a pure function and adds 4 regression tests (RED before the guard: the two "already-unpacked" cases produce app.asar.unpacked.unpacked; GREEN after; full macOS suite 28 passing, eslint clean). Happy to fold those tests into this PR instead if you'd prefer that shape, or rebase #953 on top of this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

helperPath replace produces 'app.asar.unpacked.unpacked/…' when caller is itself in app.asar.unpacked → posix_spawnp ENOENT

2 participants