Display '-patched' by version in about dialog and issue reporter when impure

[RMacfarlane]

#56929

When source code has been tampered with, adds '-patched' to the version information sent in the issue reporter and in the about dialog. Removes '[UNSUPPORTED]' from the titlebar.

Since the IntegrityService is not available when the WindowsService is constructed, I've added an _isPure flag to the WindowsService that gets updated after a window is created.

[bpasero]

@RMacfarlane I would not suggest to remove the title change, otherwise we make it even less visible that people run a patched version. I would also make the issue reporter inform the user even stronger that the version is unsupported, possibly not even allowing to file an issue in the first place?

[bpasero]

I also wonder if we should move the integrity check code to the main side (given it is being needed there now) and out of the renderer. I see no reason why we should check for integrity on each window that opens?

@alexandrudima for input on that suggestion.

[alexdima]

Brief history:

• people were installing an extension that was actively modifying workbench.main.css and workbench.main.js to get icons.
• we were receiving a lot of issues on GitHub where we would investigate for a long time and it turns out the problem was introduced by 3rd party patching.
• we added a trivial md5 checksum to indicate to people that they should not file issues against us because we cannot support a version patched by a 3rd party.
• the check ends up painting [Unsupported] and shows a message box saying that the software is not supported and they should not create issues to us.
• we've added the title indication to make it from screenshots if the users are using a patched version.

---

Now it turns out there is an extension in the marketplace that removes the md5 checksums to prevent the title and the popup from showing.

---

I am OK to move all the logic into the issue reporter. The issue reporter can show with big bold letters that we are not accepting issues from this installation because it is patched. The form should be disabled. Then, we can remove the title indication and even the message box... It turns out we don't get a lot of issues from the patched version anymore...

[bpasero]

@alexandrudima yeah thats an alternative which seems like it could work (move everything into the issue reporter). I do agree that we probably get most issues through there so we should make sure to prevent that at this level.

[RMacfarlane]

My only concern is that if we make it impossible to file from the issue reporter for people with patched versions, they would just go to GitHub directly and not include the information that there was patching. So perhaps we should also add a statement in the issue template about this. Otherwise I think moving all of the logic into the issue reporter is a good idea.

@fabiospampinato @Tyriar fyi

[Tyriar]

I think we should show patched in the issue reporter, the about dialog and if possible, code --version and code --help. If people report issues we could request they run code --version and paste in the output or screenshot the about page. I'm not sure if it's a lot of work to do this check inside the CLI.

[fabiospampinato]

IMHO preventing to file issues all together via the issue reporter is too user hostile, besides it's impossible to know for sure if someone was running a patched version or not. I'm personally running a patched version and I've opened quite a few issues, which in theory are somewhat valuable for the product.

Perhaps the problem should be tackled from the other way around, removing the need for patching the program because the reasons why people do it are already supported (e.g. allowing loading a custom CSS file, or #39137 (comment)).

[bpasero]

I am honest: I never pay attention to screenshots, status output or whatever for the "patched" label. If the issue reporter has a big fat warning I think that would help me more.

[Tyriar]

Perhaps the problem should be tackled from the other way around, removing the need for patching the program because the reasons why people do it are already supported (e.g. allowing loading a custom CSS file, or #39137 (comment))

@fabiospampinato but the real "fix" for that is to allow direct access to the DOM which I don't see every happening. Also remember as the extension API grows, so does the cost in maintaining it which has a direct impact on our ability to make certain changes.

For #39137, let's keep discussing over on that issue.

[fabiospampinato]

@Tyriar it would be useful to know why people are patching the program, I doubt the majority of the people are mutating the DOM other than for loading a custom CSS file 🤔 Maybe by adding support for this a big enough percentage of people will stop patching.

[TylerLeonhardt]

This is so old. I'm just going to close it.