Add support for VirusTotal

nvdaes commented 3 months ago

Issue number

Fixes issue nvaccess/addon-datastore#3246

Summary of the issue

VirusTotal may catch malware bundled with add-ons. Also, knowing the sha256 of scanned add-ons, the URL to see results at different datetimes maybe built, allowing users to see this information even before installing an add-on if this was included in the NVDA store in the future.

Development strategy

Virus Total CLI is installed when needed.
Add-ons are scanned when the submission issue is created.
Info about the add-on file is requested to Virus Total later, when the pull request is created, to give time to Virus Total to show results, trying to avoid getting empty analysis.
NV Access needs to create an API key in Virus Total.
The addonMetadata.json artifact is used to get the add-on id and sha256.
A falsePositiveAddons.json file has been added. If VirusTotal analysis fails, a pull request will be created adding the sha256 of the addon to a list associated with the add-on ID, in the falsePositiveAddons.json file.
If VirusTotal should be skipped for this add-on, NV Access will merge the created pull request, delete the branch created for the submission (in the form submitterIssueNumber), and relabel the issue to trigger a new workflow.

Testing performed

Use the sha256 of a malicious add-on, simulating that this sha correspond to a non malicious add-on.
Use the real sha256 calculated for the same add-on.

https://github.com/nvdaes/addon-datastore/issues/1299

nvdaes commented 3 months ago

Tested with a valid add-on:

https://github.com/nvdaes/addon-datastore/actions/runs/9250658201

nvdaes commented 3 months ago

Test simulating that customNotifications is a malicious add-on, hard-coding the sha256 of a flagged add-on (here we see the VirusTotal failure):

https://github.com/nvdaes/addon-datastore/actions/runs/9251468377

nvdaes commented 3 months ago

Last test: after merging the manualApproval PR, simulating that NV Access accepted this add-on as a false positive:

https://github.com/nvdaes/addon-datastore/actions/runs/9251719545

nvdaes commented 3 months ago

@seanbudd , this is ready for review.

nvdaes commented 3 months ago

Sorry, I remembered that codeQl analysis workflow needs to be updated so that reviewedAddons.json can be used for codeQl and virusTotal, and the manualApproval pull request considers both analysis. I'll submit tests now.

nvdaes commented 3 months ago

Test: virusTotal success, codeQl excluding warnings fails:

https://github.com/nvdaes/addon-datastore/actions/runs/9259669099/job/25472099153

nvdaes commented 3 months ago

Test: maualApproval PR merged after analysis failure. The submission issue is closed as completed via its PR:

https://github.com/nvdaes/addon-datastore/actions/runs/9259808380

nvdaes commented 3 months ago

Test: making VirusTotal and codeQL analysis fail, to see how a unique pull request for manual approval is created, and just an issue comment requesting to keep the submission issue opened. readFeeeds 24.0.0 and the same issue testing just codeQl failure is ised:

https://github.com/nvdaes/addon-datastore/actions/runs/9260026346

nvdaes commented 3 months ago

I think that all changes are tested. The submission issue for readFeeds 24.0.0 can be found at

https://github.com/nvdaes/addon-datastore/issues/1387

nvdaes commented 3 months ago

Si I think this is ready for review, @seanbudd

nvdaes commented 2 months ago

@seanbudd , I think all your suggestions are applied.

seanbudd commented 2 months ago

note it seems I am unable to commit/push to your fork

nvdaes commented 2 months ago

I've applied your last suggestion about sender. I'll try to grant you access to my fork.

nvdaes commented 2 months ago

@seanbudd , I have send you an invitation as an admin of my fork. Please accept it.

seanbudd commented 2 months ago

@coderabbitai review

nvaccess / addon-datastore-staging