-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for VirusTotal #61
Conversation
Tested with a valid add-on: https://github.com/nvdaes/addon-datastore/actions/runs/9250658201 |
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 |
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 |
@seanbudd , this is ready for review. |
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. |
Test: virusTotal success, codeQl excluding warnings fails: https://github.com/nvdaes/addon-datastore/actions/runs/9259669099/job/25472099153 |
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 |
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 |
I think that all changes are tested. The submission issue for readFeeds 24.0.0 can be found at |
Si I think this is ready for review, @seanbudd |
.github/workflows/sendJsonFile.yml
Outdated
git checkout -b ${{ github.event.sender.login }}${{ steps.get-data.outputs.issueNumber }} | ||
git add . | ||
git commit -m "Submit add-on" | ||
git push origin ${{ github.event.issue.user.login }}${{ steps.get-data.outputs.issueNumber }} | ||
git push origin ${{ github.event.sender.login }}${{ steps.get-data.outputs.issueNumber }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should remain as is. Please don't use the event sender
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Co-authored-by: Sean Budd <[email protected]>
@seanbudd , I think all your suggestions are applied. |
note it seems I am unable to commit/push to your fork |
Co-authored-by: Sean Budd <[email protected]>
I've applied your last suggestion about sender. I'll try to grant you access to my fork. |
@seanbudd , I have send you an invitation as an admin of my fork. Please accept it. |
@coderabbitai review |
Issue number
Fixes issue nvaccess#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
Testing performed
nvdaes#1299