Create action to virus scan all add-ons (#4302)

seanbudd · web-flow · commit dedc7cd6e71b · 2024-09-30T09:46:39.000+10:00
VirusTotal scanning has been enabled for newly submitted add-ons.
This PR adds a GitHub action to scan all already submitted add-ons with VirusTotal.

When the scan runs, a PR is opened to add scan URLs to add-on metadata, and update the list of approved add-ons which have Virus scanning flagged.
diff --git 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,13 +242,20 @@ 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
       - name: Download add-on metadata
         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 <github-actions@github.com>
           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
@@ -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
@@ -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 <github-actions@github.com>
+          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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ module.exports = ({core}, path) => {`
`23`	`23`	`reviewedAddonsData[addonId] = [];`
`24`	`24`	`}`
`25`	`25`	`reviewedAddonsData[addonId].push(sha256);`
`26`		`- const stringified = JSON.stringify(reviewedAddonsData, null, 2);`
	`26`	`+ const stringified = JSON.stringify(reviewedAddonsData, null, "\t");`
`27`	`27`	`fs.writeFileSync('reviewedAddons.json', stringified);`
`28`	`28`	`core.setFailed("Security analysis failed");`
`29`	`29`	`};`