Test nvaccess/pull/6411 (#83)

Author: seanbudd

.github/workflows/checkAndSubmitAddonMetadata.yml

@@ -252,22 +252,13 @@ jobs:
           script: |
             const setVirusTotalAnalysisStatus = require('./.github/workflows/virusTotalAnalysis.js')
             setVirusTotalAnalysisStatus({core}, ["${{ needs.getAddonId.outputs.addonFileName }}"])
-      - 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
+      - name: Commit scanned add-ons results
+        run: |
+          git add addons
+          git config user.name "github-actions"
+          git config user.email "[email protected]"
+          git commit -m "Add VirusTotal results"
+          git push --set-upstream origin ${{ env.branchName }}
       - name: Warn if analysis fails
         if: failure()
         uses: peter-evans/create-or-update-comment@v4
@@ -286,47 +277,20 @@ jobs:
       addonFileName: ${{ needs.getAddonId.outputs.addonFileName }}
       branchName: ${{ inputs.issueAuthorName }}${{ inputs.issueNumber }}
 
-  createManualApproval:
+  requireManualApproval:
     needs: [getAddonId, virusTotal-analysis, codeQL-analysis]
-    if: ${{ always() && (contains(needs.virusTotal-analysis.result, 'failure') || contains(needs.codeQL-analysis.result, 'failure')) }}
-    runs-on: windows-latest
-    strategy:
-      matrix:
-        python-version: [ 3.11 ]
-    permissions:
-      contents: write
-      issues: write
-      pull-requests: write
+    if: ${{ always() && (needs.virusTotal-analysis.result == 'failure' || needs.codeQL-analysis.result == 'failure') }}
+    runs-on: ubuntu-latest
+    environment:
+      # Require a manual deployment approval if security analysis fails
+      name: securityReview
     steps:
-      - name: Checkout repository
-        uses: actions/checkout@v4
-      - name: Download artifacts
-        uses: actions/download-artifact@v4
-        with:
-          merge-multiple: true
-      - name: Create pull request
-        id: cpr
-        uses: peter-evans/create-pull-request@v7
-        with:
-          add-paths: reviewedAddons.json
-          title: Add reviewed add-on (${{ needs.getAddonId.outputs.addonId }})
-          branch: reviewedAddon${{ github.event.issue.number }}
-          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.
-          author: github-actions <[email protected]>
-          delete-branch: true
-      - name: Request to keep issue opened
-        uses: peter-evans/create-or-update-comment@v4
-        with:
-          issue-number: ${{ inputs.issueNumber }}
-          body: |
-            Please, don't close this issue.
-            Wait until #${{ steps.cpr.outputs.pull-request-number }} is merged.
+      - name: Confirm approval
+        run: echo "Manual approval performed"
 
   mergeToMaster:
-    needs: [getAddonId, createPullRequest, codeQL-analysis, virusTotal-analysis]
+    needs: [getAddonId, createPullRequest, requireManualApproval]
+    if: ${{ always() && (needs.requireManualApproval.result == 'success' || needs.requireManualApproval.result == 'skipped') }}
     permissions:
       contents: write
       pull-requests: write

.github/workflows/codeql-analysis.yml

@@ -51,22 +51,22 @@ jobs:
           script: |
             const setSecurityAnalysisStatus = require('./.github/workflows/securityAnalysis.js')
             const resultsPath = 'results/python.sarif'
-            setSecurityAnalysisStatus({core}, "${{ inputs.addonFileName }}", resultsPath)
+            setSecurityAnalysisStatus({core}, "${{ inputs.addonFileName }}", resultsPath, "errors")
+      - name: Commit scanned add-ons results
+        if: always()
+        run: |
+          git add ${{ inputs.addonFileName }}
+          git config user.name "github-actions"
+          git config user.email "[email protected]"
+          git commit -m "Add CodeQL error results"
+          git push --set-upstream origin ${{ inputs.branchName }}
       - name: Upload results
         id: uploadResults
         if: failure()
         uses: actions/upload-artifact@v4
         with:
           name: results-excluding-warnings
           path: results/python.sarif
-      - name: Upload manual approval
-        id: uploadManualApproval
-        if: failure()
-        uses: actions/upload-artifact@v4
-        with:
-          name: manualApproval
-          path: reviewedAddons.json
-          overwrite: true
       - name: Warn if analysis fails
         if: failure()
         uses: peter-evans/create-or-update-comment@v4
@@ -122,7 +122,16 @@ jobs:
           script: |
             const setSecurityAnalysisStatus = require('./.github/workflows/securityAnalysis.js')
             const resultsPath = 'results/python.sarif'
-            setSecurityAnalysisStatus({core}, "${{ inputs.addonFileName }}", resultsPath)
+            setSecurityAnalysisStatus({core}, "${{ inputs.addonFileName }}", resultsPath, "warnings")
+      - name: Commit scanned add-ons results
+        if: always()
+        run: |
+          git pull --rebase
+          git add ${{ inputs.addonFileName }}
+          git config user.name "github-actions"
+          git config user.email "[email protected]"
+          git commit -m "Add CodeQL warnings results"
+          git push --set-upstream origin ${{ inputs.branchName }}
       - name: Upload results
         id: uploadResults
         if: failure()

.github/workflows/securityAnalysis.js

@@ -1,28 +1,23 @@
-module.exports = ({core}, addonMetadataPath, resultsPath) => {
+module.exports = ({core}, addonMetadataPath, resultsPath, testType) => {
   const fs = require('fs');
   const addonMetadataContents = fs.readFileSync(addonMetadataPath);
   const addonMetadata = JSON.parse(addonMetadataContents);
-  const addonId = addonMetadata.addonId;
-  const sha256 = addonMetadata.sha256;
-  const reviewedAddonsContents = fs.readFileSync('reviewedAddons.json');
-  const reviewedAddonsData = JSON.parse(reviewedAddonsContents);
-  if (reviewedAddonsData[addonId] !== undefined && reviewedAddonsData[addonId].includes(sha256)) {
-    core.info('Analysis skipped');
-    return;
-  }
   const contents = fs.readFileSync(resultsPath);
   const data = JSON.parse(contents);
   const runs = data.runs[0];
   const results = runs.results;
+  if (testType !== "errors" && testType !== "warnings") {
+    core.setFailed(`Invalid test type: ${testType}`);
+  }
+  if (addonMetadata.scanResults === undefined) {
+    addonMetadata.scanResults = {};
+  }
+  addonMetadata.scanResults[`codeQL-${testType}`] = results;
+  const stringified = JSON.stringify(addonMetadata, null, "\t");
+  fs.writeFileSync(addonMetadataPath, stringified + "\n");
   if (results.length === 0) {
     core.info("Security analysis succeeded");
-    return;
-  }
-  if (reviewedAddonsData[addonId] === undefined) {
-    reviewedAddonsData[addonId] = [];
+  } else {
+    core.setFailed("Security analysis failed");
   }
-  reviewedAddonsData[addonId].push(sha256);
-  const stringified = JSON.stringify(reviewedAddonsData, null, "\t");
-  fs.writeFileSync('reviewedAddons.json', stringified);
-  core.setFailed("Security analysis failed");
 };

.github/workflows/virusScanAllAddons.yml

@@ -23,8 +23,8 @@ jobs:
     env:
       VT_API_KEY: ${{ secrets.VT_API_KEY }}
       VT_API_LIMIT: ${{ vars.VT_API_LIMIT }}
-      BRANCH_NAME: addVTURLs${{ github.run_number }}
-      BATCH_SIZE: 100
+      BRANCH_NAME: addVTresults${{ github.run_number }}
+      BATCH_SIZE: 10
     steps:
       - name: Checkout repository
         uses: actions/checkout@v4
@@ -39,15 +39,15 @@ jobs:
         uses: actions/setup-node@v4
       - name: Install npm dependencies
         run: npm install glob uuid
-      - name: Get add-on filenames without vtScanUrl
+      - name: Get add-on filenames without scan results
         shell: bash
         run: |
           for file in ./addons/*/*.json; do
-            if (jq -r '.vtScanUrl' "$file" | grep -q 'null\|""'); then
+            if (jq -r '.scanResults | .virusTotal' "$file" | grep -q 'null\|""'); then
               echo "$file" >> addonsWithoutVT.txt
             fi
           done
-          wc -l addonsWithoutVT.txt | awk '{print "Total add-ons without VT URLs: " $1}'
+          wc -l addonsWithoutVT.txt | awk '{print "Total add-ons without scan results: " $1}'
       - name: Set Virus Total analysis status
         id: setVirusTotalAnalysisStatus
         uses: actions/github-script@v7
@@ -63,29 +63,13 @@ jobs:
           git add addons
           git config user.name "github-actions"
           git config user.email "[email protected]"
-          git commit -m "Add VirusTotal review URLs"
+          git commit -m "Add VirusTotal results"
           git push --set-upstream origin ${{ env.BRANCH_NAME }}
           gh pr create \
-          --title "Add VirusTotal review URLs" \
+          --title "Add VirusTotal results" \
           --base ${{ github.ref }} \
           --head ${{ env.BRANCH_NAME }} \
-          --body "Add VirusTotal review URLs to add-ons"
+          --body "Add VirusTotal results to add-ons"
           gh pr merge --merge "${{ env.BRANCH_NAME }}"
         env:
           GH_TOKEN: ${{ github.token }}
-      - 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

.github/workflows/virusTotalAnalysis.js

@@ -10,17 +10,16 @@ function writeVTScanUrl({core}, metadataFile, addonMetadata) {
   addonMetadata.vtScanUrl = vtScanUrl;
   stringified = JSON.stringify(addonMetadata, null, "\t");
   // Write vtScanUrl to add-on metadata file
-  fs.writeFileSync(metadataFile, stringified);
+  fs.writeFileSync(metadataFile, stringified + "\n");
   // Store the latest vtScanUrl for single file analysis
   core.setOutput("vtScanUrl", vtScanUrl);
 }
 
 
-function getVirusTotalAnalysis({core}, addonMetadata, metadataFile, reviewedAddonsData) {
+function getVirusTotalAnalysis({core}, addonMetadata, metadataFile) {
   /*
   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.
+  Store the results in the metadata file and the scan URL in the add-on metadata file.
   If Virus total fails to scan the add-on, fail the job.
   */
   countAPIUsageAndWait({core});
@@ -33,27 +32,26 @@ function getVirusTotalAnalysis({core}, addonMetadata, metadataFile, reviewedAddo
       if (core._isSingleFileAnalysis) {
         core.setFailed(`Failed to get VirusTotal analysis for ${metadataFile}`);
       }
+      // Resubmit and try again
       virusTotalSubmit({core}, [metadataFile]);
-      getVirusTotalAnalysis({core}, addonMetadata, metadataFile, reviewedAddonsData);
+      getVirusTotalAnalysis({core}, addonMetadata, metadataFile);
       return;
     }
     writeVTScanUrl({core}, metadataFile, addonMetadata);
     // Append the VirusTotal analysis to the file for an artifact
     const vtData = JSON.parse(stdout);
-    fs.appendFileSync("vt.json", stdout);
     const stats = vtData[0]["last_analysis_stats"];
     const malicious = stats.malicious;
+    if (addonMetadata.scanResults === undefined) {
+      addonMetadata.scanResults = {};
+    }
+    addonMetadata.scanResults.virusTotal = vtData;
+    stringified = JSON.stringify(addonMetadata, null, "\t");
+    fs.writeFileSync(metadataFile, stringified + "\n");
     if (malicious === 0) {
       core.info(`VirusTotal analysis succeeded for ${metadataFile}`);
-      return;
-    }
-    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) {
+    else if (core._isSingleFileAnalysis) {
       core.setFailed(`VirusTotal analysis failed for ${metadataFile}`);
     }
   });
@@ -68,20 +66,17 @@ function getVirusTotalAnalysisIfRequired({core}, metadataFile) {
   */
   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 for ${metadataFile}`);
-    return;
+  // Check if add-on has been submitted before to VirusTotal.
+  if (addonMetadata.vtScanUrl === undefined) {
+    core.info(`VirusTotal scanning has not been performed for ${metadataFile}`);
+    virusTotalSubmit({core}, [metadataFile]);
   }
-  // Check if add-on has been scanned before through VirusTotal.
-  if (addonMetadata.vtScanUrl !== undefined) {
+  // Check if add-on has had results saved before through VirusTotal.
+  if (addonMetadata.scanResults !== undefined && addonMetadata.scanResults.virusTotal !== undefined) {
     core.info(`VirusTotal analysis skipped, already performed for ${metadataFile}`);
     return;
   }
-  getVirusTotalAnalysis({core}, addonMetadata, metadataFile, reviewedAddonsData);
+  getVirusTotalAnalysis({core}, addonMetadata, metadataFile);
 }
 
 module.exports = ({core}, metadataFiles) => {