refactor: plugin sandbox composition

[luciorubeens]

Summary

Refactor all code that was in the plugin-manager.js file, separating responsibilities for better reading, scalability and testing.

Currently only one file manages reading, loading, validation, sandboxing and mounting the plugins.

Each plugin can set some permissions, each permission can perform side effects (such as adding new routes) or inject methods (such as alerts). Then these tasks were moved to the setup and sandbox folders.

Checklist

• Documentation (if necessary)
• Tests (if necessary)
• Ready to be merged

[codecov]

Codecov Report

Merging #1507 into develop will increase coverage by 3.99%.
The diff coverage is 73.76%.

@@             Coverage Diff             @@
##           develop    #1507      +/-   ##
===========================================
+ Coverage    41.32%   45.32%   +3.99%     
===========================================
  Files          240      274      +34     
  Lines         6681     6791     +110     
  Branches      1324     1311      -13     
===========================================
+ Hits          2761     3078     +317     
+ Misses        3692     3550     -142     
+ Partials       228      163      -65

Impacted Files	Coverage Δ
...ervices/plugin-manager/setup/font-awesome-setup.js	0% <0%> (ø)
...vices/plugin-manager/component/compile-template.js	0% <0%> (ø)
...vices/plugin-manager/utils/validate-plugin-path.js	0% <0%> (ø)
src/renderer/store/modules/plugin.js	0% <0%> (ø)	⬆️
...c/renderer/services/plugin-manager/plugin-setup.js	0% <0%> (ø)
...renderer/services/plugin-manager/plugin-sandbox.js	0% <0%> (ø)
...ces/plugin-manager/sandbox/font-awesome-sandbox.js	100% <100%> (ø)
...er/services/plugin-manager/sandbox/http-sandbox.js	100% <100%> (ø)
...er/services/plugin-manager/setup/webframe-setup.js	100% <100%> (ø)
.../services/plugin-manager/sandbox/events-sandbox.js	100% <100%> (ø)
... and 67 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01ed43d...8446d1c. Read the comment docs.

[alessiodf]

I had previously refrained from commenting publicly on this PR because it was a draft and I hoped my concerns that I had expressed privately to @luciorubeens would have been addressed before the PR was marked ready. Unfortunately that is not the case.

Most importantly, this refactor now forbids props in custom components. This makes it impossible to design anything but the most basic plugins and will severely limit the usefulness of the plugin system overall in my opinion. The previous sandbox accepted props in custom components so I don't understand the logic behind removing them now. Was it just an oversight or intentional? This will really cripple the plugin system if it goes ahead in this state and I will not be able to complete some plugins that I had already started because this technical limitation will prohibit their execution.

Also it seems v-html is now prohibited. If this could have been abused to inject bad tags then I understand, but was that the reason and is there an alternative way to add html content even if they must be sanitised first? It makes things a bit harder now, of course if there was a legitimate vector for abuse then it's understandable.

In addition, relative paths seem to have changed which breaks existing plugins that depend on, for example, accessing package.json as it generates errors such as Module '../../package.json' is not allowed to be required. The path is outside the border! Can this be fixed?

[alexbarnsley]

Thank you for your comments on this refactor Alessio.

It was taken out of draft so I could look at it. If there are regressions without a reason then they will be addressed (e.g. props) which I will look into shortly. The removal of props could be an oversight. If so, I will look to rectify this pending my investigation. It will be the same case for relative paths.

v-html has been blocked for ~2 months for security reasons following a bugcrowd report. I'll look into the potential of us being able to disclose the details of the report for everyone's benefit: 8e671a2#diff-05eeda514b355ab4e18508cd89cc12dcR559

It's worth noting that we still consider this a "beta" system, so there may be some breaking changes. This PR is for the better, Lucio refactored the plugin system so we can have actual unit tests to catch any issues.

[luciorubeens]

Props should not work, they worked on custom components due to a bug. So this is not a downgrade, it is a patch. So if we decide to implement it can be done in another PR, since it's a new feature (I mean it's not a reason to block this one).

About relative paths I thought this was fixed, I'll take another look. Thanks for testing 👍

[alessiodf]

@alexbarnsley regarding v-html, it definitely did work right up to this refactor (e.g. in the current develop branch). That's why I noticed it suddenly stopped working so if it was fixed 2 months ago maybe there was a fault with the implementation somehow? Not to worry though as this refactor does fix it and I understand why it should be done.

@luciorubeens - I was always under the assumption props should and were allowed in custom components. When @alexbarnsley was teaching me the ropes of building plugins and custom components he specifically demonstrated how to create a custom component using props. I maintain that they are a fundamental necessity for any non-trivial plugin and would, in the strongest possible terms, suggest that props be reimplemented prior to any production release that would otherwise remove them.

I understand it's a beta system but, and I mean this in the best way possible, it would be appreciated if existing developers could be made aware of breaking changes in advance of them happening so they can plan ahead with new versions etc. It's disheartening to see breaking changes just appear out of the blue with no warning, even in beta releases.

[alessiodf]

Also I agree that this PR is for the better regarding the plugin system. If it supported props so to not break existing support (which I really depend on! that's why I'm so vocal about it), and could address the issue of relative paths, it'd be perfect 👌🏻