Malicious extension prevention

[afonsojramos]

Rationale

Even thought there is already a deny list that tries to prevent malicious extensions from running on the user's machines, after an extension is installed third parties can just update the extension at any given time with malicious code without oversight.

This brings tremendous risk as it has been proven in the industry that it can happen.

Proposed solution

Obviously we cannot guarantee full on security without manual checks, which is completely out of question. Therefore I recommend that we provide an option to disable automatic updates/a manual update section. This way users could always validate the extensions they are adding before doing so.

[theRealPadster]

Ideas include: use a specific version/commit in the url. Save the commit ID in localstorage, and check the latest commit on load (in extension.js), and update that in localstorage as well. So we have a "version" and "latest version" or "update available" flag. That way we can show an update button.

[FlafyDev]

Ideas include: use a specific version/commit in the url. Save the commit ID in localstorage, and check the latest commit on load (in extension.js), and update that in localstorage as well. So we have a "version" and "latest version" or "update available" flag. That way we can show an update button.

Maybe it would be safer to use the hash of the js file than the commit ID?

[theRealPadster]

Maybe it would be safer to use the hash of the js file than the commit ID?

Then we would need to redownload and calculate the hash of anything installed every time it checks. Is there a reason why commit ID wouldn't be safe?

[afonsojramos]

Ideas include: use a specific version/commit in the url. Save the commit ID in localstorage, and check the latest commit on load (in extension.js), and update that in localstorage as well. So we have a "version" and "latest version" or "update available" flag. That way we can show an update button.

Maybe it would be safer to use the hash of the js file than the commit ID?

Pretty sure that you can't forge a commit id, and it is definitely easier to implement vs calculating hashes imo.

[dbolger]

Maybe we add a "this version has been manually reviewed by personA" to make sure the user knows the version has indeed been confirmed correct. If the review hasn't been performed, at that point we could inform the user that they're about to try software that has yet to be reviewed.

[CharlieS1103]

@FivePixels While I agree this would be cool, I feel like needing a GitHub file for each extension and theme as well as needing to review things kind of defeats the purpose of having the marketplace being so accessible to repo owners to add their stuff. Regardless, I'll keep this idea in mind but this would probably need to be implemented after we implement padsters idea. We could create an issue format for extension/theme owners to request a review with requirements for a description for how to navigate the code and a brief description of the functions and such, to make the reviewing process a bit easier. (Or if they have commented their code properly that works too, but I know for a fact that my extensions code tend to be super messy and personally I would not want to go near reviewing them..)

[afonsojramos]

@CharlieS1103 I understand your point, but you don't need to evaluate anyone's code, you just need to check for malicious code, which is a far easier and simpler task in my view.

[afonsojramos]

@CharlieS1103 @theRealPadster I've been thinking over this for a while and I think it is time that we move towards a centralised manifest point of view. That is, having a manifest in spicetify-extensions (a new repo to host a readme to all the extensions + extensions manifest) and spicetify-themes (that holds a readme with images to all themes + themes manifest). This way we have more granular control of what we display to end-users. The advantages are:

• We can force submitters to have one repo per extension/theme -> Solving https://github.com/spicetify/spicetify-marketplace/issues/75
• We can temporarily remove apps that have been broken for a while solving #111
• There would be no problems in terms of migrations like in #137, since we would have full control of the whole flow.
• Every time that an extension was submitted they only needed to submit a PR adding their extension and respective links to the manifest. If you prefer we can have this manifest in spicetify-cli to be more visible to extension and theme creators.
• Extensions would be developed in their own branches giving more power to creators without having the need to make PRs to the "official" repos such as spicetify-themes or spicetify-cli.

Additionally, we wouldn't have problems related to this. https://github.com/spicetify/spicetify-themes/pull/759

I really think that this is the way forward and I can help with the migration of these informing all creators and creating a base manifest holding all extensions & themes.

Let me know what you think.

[CharlieS1103]

For things such as the spicetify themes repo and such, the concept of having one repo for every theme/extension is more or less just impossible without losing a lot of extensions to laziness. Therefore, it would probably be best if we continue multi-extension/theme manifest support.

[afonsojramos]

Do you really think that people would not submit a PR out of laziness after dedicating a lot of work to creating said extension in the first place? I firmly disagree with this, especially because they already do it, except that it is within their repo. They add the manifest and the topic. If you weigh in the benefits vs disadvantages I think that a centralised manifest is better than centralised repos full of themes. Look at the Glaze theme for example, where the spicetify-themes version is severely out of date 👀

[theRealPadster]

I agree and would like to get things converted to a single manifest at some point. Things are just way to up in the air right now with the jsx rewrite and the search feature that will need to be merged over as well.

[afonsojramos]

I'm going to assign myself and try to tackle it then (or at least try to give it an initial push)!

[CharlieS1103]

@afonsojramos Didn't mean they wouldn't submit a PR, i meant that making them keep their extensions/themes in separate repos would probably be annoying for larger repos(spicetify-themes, my extension repo, and many others) So we probably shouldn't remove support for storing multiple extensions / themes in one repo.