Include description of the extension in the PR?

[fedorov]

I feel like there are more and more extensions that are submitted, and this is very exciting, but I find it difficult to quickly review what the given extension is about based on the information provided in the PR.

It is good we have a checklist, but what would be nice to have in addition to the checklist is 1) lay-person description of what the extension is and what it does; 2) link to the repository with the extension; 3) link to a paper related to the extension (if such paper available).

I understand that we have the repo link in the s4ext in the extension, but it's an extra step to look it up (and not everyone looking at the extension will know where to look it up), and it is not hyperlinked.

I think the proposed changes should not make the submission more burdensome, but will make those PRs more informative, and ultimately may raise the visibility of the proposed functionality.

[jcfr]

Regarding the metadata submitted, I will add a GitHub action extracting these from the submitted .s4ext file and posting it as a comment to the PR.

[lassoan]

I agree that it is an extra step to look these up. For this reason, I don't even attempt to review a PR on my phone etc.

Asking the submitter to manually copy some details would not help much because we actually need to check if the source (s4ext file, source code, website) are correct. These are all also expected to change (even the extension name, repository URL, etc. is often changed as a result of the review process).

@jcfr's suggestion for automatically extracting (and maybe also validating, comparing if the content matches, valid, etc.) using GitHub actions could help. It would be necessary to be able to regenerate this report at any time, because most of the files are outside the ExtensionsIndex repository, so the GitHub action would not be triggered automatically.

I'm also wondering if we are trying to do too much. Maybe we could just let anyone submit almost anything and just but assign a trust level to each extension (0 = not reviewed, 1 = passed basic review, experimental, ... 4 = commonly used, well maintained, essential extension) to let users know what they can expect. We could filter the extensions manager to show TL >=1 extensions by default.

[jamesobutler]

Maybe we could just let anyone submit almost anything

Would this further open up the factory build machine to security risks? The build machine already has extensions building based on a branch name which allows for the maintainers of those repos to push anything which could include malicious execution of scripts on the factory build machines? @jcfr Is there anything being done to mitigate this risk?

[pieper]

In addition to the build machines, which probably don't have any valuable data anyway, allowing unreviewed extensions to be installed on user machines is a big security risk too since they are running with the user's privileges.

[lassoan]

PyPI is free for all, without any security or other audit whatsoever. Therefore, even if we review a package and even if we lock the extension git hash, somebody can still sneak malicious content. It is of course a universal problem that all app stores suffer from. Slicer is not worse than for example the Chrome extensions store. Let's talk about it more at today's weekly meeting.

[jamesobutler]

Sorry I’m unable to attend the weekly meeting today though I would be interested in what you all discuss. If there is less review of extension submissions that then results in malicious code, does the extension store have liability as well (Kitware since packages are served from their server) in addition to the malicious actors?

Detailing first party extensions could help to promote trust when browsing the extensions manager. These would be extensions within the Slicer GitHub organization.

[lassoan]

If there is less review of extension submissions that then results in malicious code

Less review is extremely unlikely to result in more malicious code. More malicious code will appear if there are more malicious actors.

does the extension store have liability as well

We keep following standard industry practices here: zero liability. Maybe we should show a disclaimer in the extension manager to clarify this?

Detailing first party extensions could help to promote trust

I agree this could be the key: provide information on how much we trust the developers. It could be a distinct score from the extension's quality (e.g., modules save their state into the scene) or maturity (e.g., algorithms are validated), or we could have a combined metric for all these qualities.

[fedorov]

Maybe we could just let anyone submit almost anything

I have to say this discussion veered off quite far from the initial topic! I personally do not see why it would be important to let anyone submit just anything, and I do not see why it would be a burden at all to write few sentences about what the extension is doing. If that is too hard, what can be expected in terms of maintenance of the extension?

[lassoan]

We need to decide if we want to simplify the review process. If we choose to simplify by deciding based on the submitted metadata then it makes sense to make all metadata conveniently accessible in the pull request, because the whole decision could just take a few minutes. If we decide to keep extension reviews as thorough as they are currently (taking 4-8 hours including initial review and follow-ups with developers), then we can close this issue right now, because saving a few minutes in the review process does not matter.

The underlying issue is that we are getting more extension submissions that we do not have time to thoroughly review and bring up to current standards. We either discourage developers from contributing by our long response time or we let low-quality extensions published in the app store. I think discouraging developers is very bad, so I'm trying to see if it was possible to let in low-quality extensions without hurting users.

[pieper]

I really wish people could distribute extensions without going through the extension index. It puts us in the loop for every project in a way that's different from the github, npm, and pypi model.

[fedorov]

@lassoan thank you for the response. I now understand how poorly timed my suggestion was. I most definitely appreciate it takes huge effort to review the submissions. I didn't mean to make your work more complicated!

I have to say that, at the same time, if the PR has a description of the extension, it would perhaps be more likely for someone who is not usually reviewing extension to be motivated to spend time testing it, if it is within the are of expertise or interest for that person. So it potentially could help you. For example, if I see an extension related to prostate cancer (or other applications where we have interest and expertise), I would be more than willing to ask the engineer working with me to spend time testing it and summarizing experience, or even do it myself.