diff --git a/.github/workflows/checkAndSubmitAddonMetadata.yml b/.github/workflows/checkAndSubmitAddonMetadata.yml index 35abceee619..895b171a5d2 100644 --- a/.github/workflows/checkAndSubmitAddonMetadata.yml +++ b/.github/workflows/checkAndSubmitAddonMetadata.yml @@ -66,7 +66,7 @@ jobs: script: | const addonId = "${{ steps.getAddonId.outputs.result }}" return addonId.replace(/[^a-zA-Z0-9]/g, "") - - name: Copy add-on metadata file + - name: Copy add-on metadata file run: | Copy-Item ${{ steps.getAddonFileName.outputs.result }} addonMetadata.json - name: Upload add-on @@ -155,6 +155,7 @@ jobs: issues: write outputs: pullRequestNumber: ${{ steps.cpr.outputs.pull-request-number }} + addonFileName: ${{ steps.getAddonFileName.outputs.addonFileName }} steps: - name: Checkout code uses: actions/checkout@v4 @@ -188,6 +189,7 @@ jobs: repository: nvaccess/addon-datastore-validation path: validation submodules: true + ref: addVtScanUrl - name: Install addon-datastore-validation dependencies run: | python -m pip install --upgrade wheel @@ -240,6 +242,9 @@ jobs: issues: write env: VT_API_KEY: ${{ secrets.VT_API_KEY }} + VT_API_LIMIT: ${{ vars.VT_API_LIMIT }} + outputs: + vtScanUrl: ${{ steps.setVirusTotalAnalysisStatus.outputs.vtScanUrl }} steps: - name: Checkout repository uses: actions/checkout@v4 @@ -247,6 +252,10 @@ jobs: uses: actions/download-artifact@v4 with: name: addonMetadata + - name: Install Node.js + uses: actions/setup-node@v2 + - name: Install glob + run: npm install glob - name: Install virusTotal run: choco install vt-cli - name: Set Virus Total analysis status @@ -255,7 +264,7 @@ jobs: with: script: | const setVirusTotalAnalysisStatus = require('./.github/workflows/virusTotalAnalysis.js') - setVirusTotalAnalysisStatus({core}) + setVirusTotalAnalysisStatus({core}, "${{ needs.createPullRequest.outputs.getAddonFileName }}") - name: Upload results id: uploadResults if: failure() @@ -279,7 +288,7 @@ jobs: issue-number: ${{ inputs.issueNumber }} body: | VirusTotal has flagged this add-on as malicious. - You can open this link and [see the results of the analysis](${{ steps.setVirusTotalAnalysisStatus.outputs.analysisUrl }}). + You can open this link and [see the results of the analysis](${{ steps.setVirusTotalAnalysisStatus.outputs.vtScanUrl }}). Please contact the flagged security vendors to get them to review and unflag the false positive. Please ask here or email info@nvaccess.org if you need assistance with this process. codeQL-analysis: @@ -313,7 +322,7 @@ jobs: commit-message: Add reviewed add-on (${{ needs.getAddonId.outputs.addonId }}) body: | This add-on needs to be reviewed by NV Access due to analysis failure. - Review ${{ inputs.issueNumber }} for more information. + Review #${{ inputs.issueNumber }} for more information. author: github-actions delete-branch: true - name: Request to keep issue opened @@ -340,12 +349,12 @@ jobs: GH_TOKEN: ${{ github.token }} run: | gh pr merge ${{ inputs.issueAuthorName }}${{ inputs.issueNumber }} -b '[Automated] Merged ${{ needs.getAddonId.outputs.addonFileName }} into master (PR #${{ needs.createPullRequest.outputs.pullRequestNumber }})' -m - + createReviewComment: # jq for windows has issues parsing multiline strings (e.g. CRLF), # use linux instead. runs-on: ubuntu-latest - needs: [getAddonId, mergeToMaster] + needs: [getAddonId, mergeToMaster, virusTotal-analysis] strategy: matrix: python-version: [ 3.11 ] @@ -399,13 +408,13 @@ jobs: .[\"$addonId\"].discussionId = \"$discussionId\" | .[\"$addonId\"].discussionUrl = \"$discussionUrl\" " - + mv discussions.json discussions.old.json jq -e --tab "$jqCode" discussions.old.json > discussions.json jqExitCode=$? rm discussions.old.json exit $jqExitCode - - name: Add discussion URL to metadata + - name: Add discussion and VT scan URL to metadata if: always() run: | addonFilename=$( @@ -415,17 +424,26 @@ jobs: echo ${{ needs.getAddonId.outputs.addonId }} ) reviewUrl=$( - jq ".\"$addonId\".discussionUrl" discussions.json + jq --tab ".\"$addonId\".discussionUrl" discussions.json ) - jqCode=" + vtScanUrl=$( + echo ${{ needs.virusTotal-analysis.outputs.vtScanUrl }} + ) + jqReviewCode=" .[\"reviewUrl\"] = $reviewUrl " - + jqVTCode=" + .[\"reviewUrl\"] = $reviewUrl + " + mv $addonFilename $addonFilename.old.json - jq -e --tab "$jqCode" $addonFilename.old.json > $addonFilename - jqExitCode=$? + jq -e --tab "$jqReviewCode" $addonFilename.old.json > $addonFilename + jqReviewExitCode=$? + mv $addonFilename $addonFilename.old.json + jq -e --tab "$jqVTCode" $addonFilename.old.json > $addonFilename + jqVTExitCode=$? rm $addonFilename.old.json - exit $jqExitCode + exit !(( $jqVTExitCode || $jqReviewExitCode )) - name: Commit and push if: always() run: | diff --git a/.github/workflows/securityAnalysis.js b/.github/workflows/securityAnalysis.js index 5efd024efb7..120d74e8663 100644 --- a/.github/workflows/securityAnalysis.js +++ b/.github/workflows/securityAnalysis.js @@ -23,7 +23,7 @@ module.exports = ({core}, path) => { reviewedAddonsData[addonId] = []; } reviewedAddonsData[addonId].push(sha256); - const stringified = JSON.stringify(reviewedAddonsData, null, 2); + const stringified = JSON.stringify(reviewedAddonsData, null, "\t"); fs.writeFileSync('reviewedAddons.json', stringified); core.setFailed("Security analysis failed"); }; diff --git a/.github/workflows/virusScanAllAddons.yml b/.github/workflows/virusScanAllAddons.yml new file mode 100644 index 00000000000..99fa267ca60 --- /dev/null +++ b/.github/workflows/virusScanAllAddons.yml @@ -0,0 +1,68 @@ +name: Scan a batch of submitted add-ons with Virus Total + +on: + workflow_dispatch: + +jobs: + virusTotal-analysis: + runs-on: windows-latest + strategy: + matrix: + python-version: [ 3.11 ] + permissions: + contents: write + pull-requests: write + env: + VT_API_KEY: ${{ secrets.VT_API_KEY }} + VT_API_LIMIT: ${{ vars.VT_API_LIMIT }} + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + ref: ${{ inputs.headRef }} + - name: Install virusTotal + run: choco install vt-cli + - name: Install Node.js + uses: actions/setup-node@v2 + - name: Install npm dependencies + run: npm install glob uuid + - name: Submit add-ons with VirusTotal + uses: actions/github-script@v7 + with: + script: | + const virusTotalSubmit = require('./.github/workflows/virusTotalSubmit.js') + virusTotalSubmit({core}, "./addons/*/*.json") + - name: Set Virus Total analysis status + if: always() + id: setVirusTotalAnalysisStatus + uses: actions/github-script@v7 + with: + script: | + const setVirusTotalAnalysisStatus = require('./.github/workflows/virusTotalAnalysis.js') + setVirusTotalAnalysisStatus({core}, "./addons/*/*.json") + - name: Create PR for updated VT urls + if: always() + uses: peter-evans/create-pull-request@v6 + with: + title: Add VirusTotal review URLs + branch: addVTURLs${{ github.run_number }} + commit-message: Add VirusTotal review URLs + body: "Add VirusTotal review URLs to add-ons" + author: github-actions + add-paths: 'addons/*/*.json' + - name: Upload results + id: uploadResults + if: failure() + uses: actions/upload-artifact@v4 + with: + name: VirusTotal + path: vt.json + overwrite: true + - name: Upload manual approval + id: uploadManualApproval + if: failure() + uses: actions/upload-artifact@v4 + with: + name: manualApproval + path: reviewedAddons.json + overwrite: true diff --git a/.github/workflows/virusTotalAPISleepAndCount.js b/.github/workflows/virusTotalAPISleepAndCount.js new file mode 100644 index 00000000000..0ca1c136262 --- /dev/null +++ b/.github/workflows/virusTotalAPISleepAndCount.js @@ -0,0 +1,18 @@ +function sleep(sleepTimeMs) { + /* Sleep for sleepTimeMs milliseconds. + Atomics.wait waits a timeout of sleepTimeMs. + https://stackoverflow.com/a/56406126/8030743 + */ + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, sleepTimeMs); +} + + +module.exports = ({core}) => { + if (core._apiUsageCount >= Number(process.env.VT_API_LIMIT)) { + core.info("VirusTotal API usage limit reached"); + throw new Error("VirusTotal API usage limit reached"); + } + // Sleep 20 seconds to avoid rate limiting + sleep(20 * 1000); + core._apiUsageCount++; +} diff --git a/.github/workflows/virusTotalAnalysis.js b/.github/workflows/virusTotalAnalysis.js index 7bd4177e85f..6e0e6958d1d 100644 --- a/.github/workflows/virusTotalAnalysis.js +++ b/.github/workflows/virusTotalAnalysis.js @@ -1,38 +1,91 @@ -module.exports = ({core}) => { - const fs = require('fs'); - const { exec } = require('child_process'); - const addonMetadataContents = fs.readFileSync('addonMetadata.json'); - const addonMetadata = JSON.parse(addonMetadataContents); - const addonId = addonMetadata.addonId; - core.setOutput('addonId', addonId); - const sha256 = addonMetadata.sha256; - const analysisUrl = `https://www.virustotal.com/gui/file/${sha256}`; - console.log(analysisUrl); - core.setOutput('analysisUrl', analysisUrl); - const reviewedAddonsContents = fs.readFileSync('reviewedAddons.json'); - const reviewedAddonsData = JSON.parse(reviewedAddonsContents); - if (reviewedAddonsData[addonId] !== undefined && reviewedAddonsData[addonId].includes(sha256)) { - core.info('VirusTotal analysis skipped'); - return; - } - exec(`vt file ${sha256} -k ${process.env.VT_API_KEY} --format json`, (err, stdout, stderr) => { - console.log(`err: ${err}`); - console.log(`stdout: ${stdout}`); - console.log(`stderr: ${stderr}`); +const glob = require("glob"); +const fs = require("fs"); +const { exec } = require("child_process"); +const countAPIUsageAndWait = require("./virusTotalAPISleepAndCount"); + + +function writeVTScanUrl({core}, metadataFile, addonMetadata) { + const vtScanUrl = `https://www.virustotal.com/gui/file/${addonMetadata.sha256}`; + addonMetadata.vtScanUrl = vtScanUrl; + stringified = JSON.stringify(addonMetadata, null, "\t"); + // Write vtScanUrl to add-on metadata file + fs.writeFileSync(metadataFile, stringified); + // Store the latest vtScanUrl for single file analysis + core.setOutput("vtScanUrl", vtScanUrl); +} + + +function getVirusTotalAnalysis({core}, addonMetadata, metadataFile, reviewedAddonsData) { + /* + Get the VirusTotal analysis for the add-on file. + If the add-on is flagged as malicious, store the sha256 hash in reviewedAddons.json. + Always store the scan URL in the add-on metadata file. + If Virus total fails to scan the add-on, fail the job. + */ + countAPIUsageAndWait({core}); + exec(`vt file ${addonMetadata.sha256} -k ${process.env.VT_API_KEY} --format json`, (err, stdout, stderr) => { + if (stderr !== "" || err !== null) { + console.log(`err: ${err}`); + console.log(`stdout: ${stdout}`); + console.log(`stderr: ${stderr}`); + if (core._isSingleFileAnalysis) { + core.setFailed("Failed to get VirusTotal analysis"); + } + return; + } + writeVTScanUrl({core}, metadataFile, addonMetadata); + // Append the VirusTotal analysis to the file for an artifact const vtData = JSON.parse(stdout); - fs.writeFileSync('vt.json', stdout); + fs.appendFileSync("vt.json", stdout); const stats = vtData[0]["last_analysis_stats"]; const malicious = stats.malicious; if (malicious === 0) { - core.info('VirusTotal analysis succeeded'); + core.info("VirusTotal analysis succeeded"); return; } - if (reviewedAddonsData[addonId] === undefined) { - reviewedAddonsData[addonId] = []; + if (reviewedAddonsData[addonMetadata.addonId] === undefined) { + reviewedAddonsData[addonMetadata.addonId] = []; + } + reviewedAddonsData[addonMetadata.addonId].push(addonMetadata.sha256); + stringified = JSON.stringify(reviewedAddonsData, null, "\t"); + fs.writeFileSync("reviewedAddons.json", stringified); + if (core._isSingleFileAnalysis) { + core.setFailed("VirusTotal analysis failed"); } - reviewedAddonsData[addonId].push(sha256); - stringified = JSON.stringify(reviewedAddonsData, null, 2); - fs.writeFileSync('reviewedAddons.json', stringified); - core.setFailed('VirusTotal analysis failed'); + }); +} + + +function getVirusTotalAnalysisIfRequired({core}, metadataFile) { + /* + If we have scanned and stored the VirusTotal analysis for the add-on before, + skip the analysis. Otherwise, get the VirusTotal analysis and store the URL + in the add-on metadata. + */ + const addonMetadataContents = fs.readFileSync(metadataFile); + const addonMetadata = JSON.parse(addonMetadataContents); + const addonId = addonMetadata.addonId; + const reviewedAddonsContents = fs.readFileSync("reviewedAddons.json"); + const reviewedAddonsData = JSON.parse(reviewedAddonsContents); + // Check if add-on has been flagged before through VirusTotal. + if (reviewedAddonsData[addonId] !== undefined && reviewedAddonsData[addonId].includes(sha256)) { + core.info("VirusTotal analysis skipped, already performed"); + return; + } + // Check if add-on has been scanned before through VirusTotal. + if (addonMetadata.vtScanUrl !== undefined) { + core.info("VirusTotal analysis skipped, already performed"); + return; + } + getVirusTotalAnalysis({core}, addonMetadata, metadataFile, reviewedAddonsData); +} + +module.exports = ({core}, globPattern) => { + var metadataFiles = glob.globSync(globPattern); + // Count API usages to adhere to rate limiting + core._apiUsageCount = 0; + core._isSingleFileAnalysis = metadataFiles.length == 1; + metadataFiles.forEach(metadataFile => { + getVirusTotalAnalysisIfRequired({core}, metadataFile); }); }; diff --git a/.github/workflows/virusTotalSubmit.js b/.github/workflows/virusTotalSubmit.js new file mode 100644 index 00000000000..238e2a9990c --- /dev/null +++ b/.github/workflows/virusTotalSubmit.js @@ -0,0 +1,108 @@ +const glob = require("glob"); +const { v4: uuidv4 } = require("uuid"); +const fs = require("fs"); +const { exec } = require("child_process"); +const countAPIUsageAndWait = require("./virusTotalAPISleepAndCount"); + + +function submitAddon({core}, addonMetadata, downloadFileName) { + /* + Submit add-on file to VirusTotal for scanning. + Removes downloaded add-on file after scanning. + Later we will confirm the virus scan results with another API call. + If add-on submission fails we will continue. + */ + countAPIUsageAndWait({core}); + // scan downloaded file + exec( + `vt scan file -k ${process.env.VT_API_KEY} ${downloadFileName}`, + // increase maxBuffer size to 10GB as add-on files can be large + { maxBuffer: 1024 * 1024 * 1024 * 10 }, + (err, stdout, stderr) => { + removeDownloadedAddonFile(downloadFileName, addonMetadata.URL); + if (stderr !== "" || err !== null) { + console.log(`err: ${err}`); + console.log(`stdout: ${stdout}`); + console.log(`stderr: ${stderr}`); + console.error(`Failed to scan add-on file: ${addonMetadata.URL}`); + return; + } + }) +} + + +function removeDownloadedAddonFile(downloadFileName, metadataFile) { + exec(`rm "${downloadFileName}"`, (err, stdout, stderr) => { + // stdout is garbage here, so we don't use it + if (stderr !== "" || err !== null) { + console.log(`err: ${err}`); + console.log(`stderr: ${stderr}`); + console.error(`Failed to delete downloaded add-on file for ${metadataFile}`); + return; + } + }) +} + + +function downloadAndSubmitAddon({core}, addonMetadata) { + /* + Download add-on file to send to VirusTotal for scanning. + Removes downloaded add-on file after scanning. + Later we will confirm the virus scan results with another API call. + If add-on download or submission fails we will continue. + */ + // We need a unique name otherwise we could overwrite files + const downloadFileName = `${uuidv4()}.nvda-addon`; + exec( + `curl --fail --silent --show-error --location --output "${downloadFileName}" "${addonMetadata.URL}"`, + // increase maxBuffer size to 10GB as add-on files can be large + { maxBuffer: 1024 * 1024 * 1024 * 10 }, + (err, stdout, stderr) => { + if (stderr !== "" || err !== null) { + console.log(`err: ${err}`); + console.log(`stdout: ${stdout}`); + console.log(`stderr: ${stderr}`); + console.error(`Failed to download add-on file: ${addonMetadata.URL}`); + return; + } + submitAddon({core}, addonMetadata, downloadFileName); + }) +} + + +function submitAddonIfNotScanned({core}, metadataFile) { + /* + Check if add-on has been scanned before through VirusTotal. + If not, download and submit the add-on. + */ + const addonMetadataContents = fs.readFileSync(metadataFile); + const addonMetadata = JSON.parse(addonMetadataContents); + const sha256 = addonMetadata.sha256; + countAPIUsageAndWait({core}); + // Check if file has been scanned before + exec(`vt file ${sha256} -k ${process.env.VT_API_KEY} --format json`, (err, stdout, stderr) => { + core.debug(`stdout: ${stdout}`); + core.debug(`stderr: ${stderr}`); + core.debug(`err: ${err}`); + try { + const vtData = JSON.parse(stdout); + core.debug(`Add-on file ${metadataFile} already submitted, results: ${vtData}`); + return; + } catch (e) { + core.debug(`Add-on file ${metadataFile} has not been scanned before`); + // File has not been scanned before, + // download and submit add-on file + downloadAndSubmitAddon({core}, addonMetadata); + } + }); +} + + +module.exports = ({core}, globPattern) => { + const metadataFiles = glob.globSync(globPattern); + // Count API usages to adhere to rate limiting + core._apiUsageCount = 0; + metadataFiles.forEach(metadataFile => { + submitAddonIfNotScanned({core}, metadataFile); + }); +};