Security: Don't install arbitrary binary on user system

[iyerusad]

An extension shouldn't download binaries from arbitrary sources:

https://github.com/mvdan/sh/releases/download/v2.6.4/shfmt_v2.6.4_windows_amd64.exe is arbitrary and out of the extension's control. This would be different if this extension is being published by @mdvan. Additionally, no one should trust extension didn't modify the URL to their own payload. [1] [2] [3]

Most systems run behind a package system (brew, chocolately, apt etc), which incorporate their own flows for getting packages to a system and safeguards (e.g. checksumming executable etc).

Instead, consider outputting instructions on how to safely source shfmt, for example:

shfmt doesn't appear to be available! We checked your $PATH, C:\bin, and C:\Windows\system32 and couldn't find shfmt.exe.

Based on detected OS (Windows), here are some options to make shfmt available:

• Follow shfmt github instructions:
  https://github.com/mvdan/sh#shfmt

• Download using chocolatey:
  https://chocolatey.org/search?q=shfmt

• Download directly from https://github.com/mvdan/sh/releases and save into C:\bin\shfmt.exe (or anywhere in $PATH)

Bug: Detecting existing shfmt

The logic to identify if shfmt is available on local machine/path is broken - it didn't detect the installed/available version of shfmt on the system.

---

I understand what this extension is trying to do (make it easy for this extension to work), but its breaking some cardinal security rules. Implementing some of the safety check (checksum executable) would not change this - extensions shouldn't source arbitrary binaries.

[foxundermoon]

Out of the box and to manually configure a tradeoff

In order to meet the user's ease of use, I tried to add an automatic download function.

All code in the plugin is open source, public, and auditable. The binaries source URL is also pointing to the author's release page.

---

But yours is a good suggestion

Before the automatic download, we can add the user approval process and add the hash check of the downloaded file.user can choose auto download or manual download and config.

[foxundermoon]

Bug: Detecting existing shfmt

The logic to identify if shfmt is available on local machine/path is broken - it didn't detect the installed/available version of shfmt on the system.

I understand what this extension is trying to do (make it easy for this extension to work), but its breaking some cardinal security rules. Implementing some of the safety check (checksum executable) would not change this - extensions shouldn't source arbitrary binaries.

In order to ensure that the version of the binary file is upgraded with the plugin, the future version will not check the PATH. If you need to specify it manually, you can configure the shellformat.path and specify the version update in the changelog.

[iyerusad]

On the positive side, placing within extension folder is at least reasonable place to store it (as opposed to system bin). This is a reasonable compromise.

I would strongly suggest taking advantage of established paradigms (e.g. $PATH), but I understand you are seeing shfmt is not widely available on user systems and they are complaining about the extension not working. Frustrating to read those reviews.

Something to think about - Which are you building?

• package manager's functionality within VS Code extension (but only for one package)?
• extension to hook up shfmt to VS Code?

Closing this issue - I think existing behavior is inline with other VS Code extensions (including Microsoft's Azure extensions - they also download binaries, but they do own/build them)

[foxundermoon]

I also tried to package the files into the plugin and package them into different plugins according to different platforms, but vscode does not have such a mechanism.

[iyerusad]

package the files into the plugin...into different plugins according to different platforms
vscode does not have such a mechanism.

You wouldn't want to do that. Taking advantage of a concept like $PATH is one way that would allow you to maintain one extension that would work on various operating systems and environments. This allows you as the extension developer to focus on the extension, and operating system developers to focus on integrating a functioning $PATH (or whatever concept you utilize to achieve portability across platforms).

If you go esoteric, you might find people running VS Code on atypical architectures: ARM, within a browser, or remote shell sessions - its nice if extension works in all those scenarios.

That being said, I have seen a major multi-million (billion?) invested architecture (e.g. ARM) have a global total of less than 10 active users.... So I wouldn't be too focused on being that portable.

[foxundermoon]

thanks suggest