heroku / heroku/cli

testing for current shell uses incorrect environment variable

Open
#2,014 2 comments 0 reactions 0 assignees View on GitHub
on-call reviewed
Dominant language
TypeScript
Stars
889
Forks
236
Avg merge
2d 10h
Merged PRs (30d)
33

Description

What is the current behavior?
-----------------------------
regardless of what your current shell is, `heroku autocomplete --refresh-cache` fails if your default shell is not supported.

for example:

* my default shell is fish. fish is not supported.
* i switch to bash which is supported, but leave my default shell as fish.
* I try and run the command above. I get an error that fish shell is not supported despite the fact that i'm not _in_ fish.

What is the expected behavior?
------------------------------
when a user is running a supported shell they should never get an error about some other shell not being supported.

To put it another way, the function that test for supported shells should be testing the _current_ shell not the _default_ shell.

> Please mention your heroku and OS version.

* heroku/7.60.2 darwin-x64 node-v14.19.0
* macOS 12.4

> Please state if you are behind an HTTP proxy or company firewall as well.

yes, but that's not relevant.

## BUG SOURCE
I believe the bug originates in the `_shell()` function in `config.js` which is using the wrong environment variable to test the current shell.

```
_shell() {
let shellPath;
/// BUG HERE vvvvvvvvvvvvvvvvvvvvv
const { SHELL, COMSPEC } = process.env;
/// BUG THERE ^^^^^^^^^^^^^^^^^
if (SHELL) {
shellPath = SHELL.split('/');
}
else if (this.windows && COMSPEC) {
shellPath = COMSPEC.split(/\\|\//);
}
else {
shellPath = ['unknown'];
}
return shellPath[shellPath.length - 1];
}
```

Specifically, the problem is that the `SHELL` environment variable returns the _default_ shell not the _current_ shell. If you want the _current_ shell you should ask for the `0` (zero) environment variable. In `bash` and `zsh` this returns `bash` or `zsh` respectively. in fish it returns nothing.

[edit: after further testing I don't think that _that_ function is the source of _my_ problem (at least not with autocomplete) but it is definitely going to be the source of problems and both instances of that function need to be corrected.

example in use
```
❯ bash # switching to bash
➜ echo $0
bash
➜ zsh # switching to zsh
% echo $0
zsh
```

according to this line

```
plugin-autocomplete/lib/base.js
18: if (!['bash', 'zsh'].includes(shell)) {
```
those are you only two supported shells, so the `0` environment variable should be sufficient for your tests. However... it doesn't appear that `process.env["0"]` returns anything so... you may have to get at that info in a different way. More info on this problem can be found [in this stack overflow answer](https://stackoverflow.com/a/3327022/13973) but i haven't figured out what the idiomatic node way of obtaining this information is.

Regardless, `SHELL` is absolutely _not_ the correct way to access the information you're looking for.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.