electron-userland / electron-userland/electron-builder

Linux: the elevation dialog shows /bin/bash -c … instead of the install command

Open
#10,093 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
14.7k
Forks
1.9k
Avg merge
3d 12h
Merged PRs (30d)
48

Description

* **Electron-Builder Version**: 26.15.3 — the code below is quoted from `master` (27.0.0-alpha.7), where it is unchanged
* **Electron Updater (as-needed)**: 6.8.9 (and 7.0.0-alpha.6)
* **Node Version**: the one bundled with Electron 42.5.0
* **Electron Version**: 42.5.0
* **Platform & Target**: Linux (Ubuntu 24.04, GNOME 46, Wayland), `deb` target — the same code path serves `rpm` and `pacman`
* **Debug Logs**: see the dialog text and the Node warning below

### What happens

The privileged command is wrapped in `/bin/bash -c '…'`
([LinuxUpdater.ts#L54](https://github.com/electron-userland/electron-builder/blob/0a0c683d81993a63d4c64dfa67e42fa6fc41267d/packages/electron-updater/src/LinuxUpdater.ts#L54)), so the authentication dialog shows the wrapper instead of the
command the user is being asked to authorize:

```
Authentication is needed to run '/bin/bash -c dpkg -i /home/…/pending/App-1.2.3.deb' as the super user
```

polkit elides the middle of long strings, so on a real updater cache path a good part of what remains legible
is the wrapper rather than the package being installed.

Two consequences beyond readability:

- the wrapper is what forces the manual metacharacter escaping in `installerPath`
([LinuxUpdater.ts#L20-L34](https://github.com/electron-userland/electron-builder/blob/0a0c683d81993a63d4c64dfa67e42fa6fc41267d/packages/electron-updater/src/LinuxUpdater.ts#L20-L34)), which is documented as not supporting paths that
contain a single quote
- `spawnSyncLog` passes an args array together with `shell: true`
([BaseUpdater.ts#L299-L306](https://github.com/electron-userland/electron-builder/blob/0a0c683d81993a63d4c64dfa67e42fa6fc41267d/packages/electron-updater/src/BaseUpdater.ts#L299-L306)), so recent Node versions warn:

```
(node:538599) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead
to security vulnerabilities, as the arguments are not escaped, only concatenated.
```

### Suggested direction

`pkexec` and `sudo` accept an argv array, so the command can be spawned without a shell at all: the dialog
then shows `dpkg -i /path/app.deb`, the path needs no escaping, and `shell: true` disappears along with the
deprecation warning. `gksudo`, `kdesudo` and `beesu` take the command as a single string and would keep the
joined form.

One caveat worth deciding on: `determineSudoCommand` tries `gksudo` and `kdesudo` *before* `pkexec`, and
`pkexec` is the only one of the four that takes argv — so on a machine that still has either of them the
dialog would be unchanged. Both were removed from Debian (since Buster) and Ubuntu (since 18.04), the Debian
maintainers removing gksu as unsafe, and neither works under Wayland.

This depends on #10092: spawning without a shell requires the argv-based spawn that the asynchronous install
path introduces. I have a patch for both and will open the PRs shortly.

Contributor guide

Open the contributing guide

Research direction

Start with packages/electron-updater/src/LinuxUpdater.ts, especially determineSudoCommand and the installerPath handling at lines 20-54, then inspect spawnSyncLog in BaseUpdater.ts at lines 299-306. Review the argv-based install work in #10092 and compare the command forms accepted by pkexec, sudo, gksudo, kdesudo, and beesu. Done means the elevation dialog shows the install command and the Node shell warning and unnecessary path escaping are gone without changing supported sudo backends.

Written by the indexing model from the issue text.

Assessment

Tech stack
electron, node.js, typescript
Domain
desktop, operating-systems, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.