Skip to content
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

CodeReviewID appears twice and upload fails with "contains duplicate item" #3865

Closed
faximan opened this issue Feb 8, 2024 · 2 comments
Closed
Labels
kind/bug Something isn't working

Comments

@faximan
Copy link

faximan commented Feb 8, 2024

Describe the bug
Using the default scorecard.yaml on a private repo with a PAT.

# This workflow uses actions that are not certified by GitHub. They are provided
# by a third-party and are governed by separate terms of service, privacy
# policy, and support documentation.

name: Scorecard supply-chain security
on:
  # For Branch-Protection check. Only the default branch is supported. See
  # https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection
  branch_protection_rule:
  # To guarantee Maintained check is occasionally updated. See
  # https://github.com/ossf/scorecard/blob/main/docs/checks.md#maintained
  schedule:
    - cron: '45 7 * * 5'
  push:
    branches: [ "main" ]

# Declare default permissions as read only.
permissions: read-all

jobs:
  analysis:
    name: Scorecard analysis
    runs-on: ubuntu-latest
    permissions:
      # Needed to upload the results to code-scanning dashboard.
      security-events: write
      # Needed to publish results and get a badge (see publish_results below).
      id-token: write
      # Needed because installing in a private repository.
      contents: read
      actions: read
      # To allow GraphQL ListCommits to work
      issues: read
      pull-requests: read
      # To detect SAST tools
      checks: read

    steps:
      - name: "Checkout code"
        uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
        with:
          persist-credentials: false

      - name: "Run analysis"
        uses: ossf/scorecard-action@e38b1902ae4f44df626f11ba0734b14fb91f8f86 # v2.1.2
        with:
          results_file: results.sarif
          results_format: sarif
          repo_token: ${{ secrets.SCORECARD_TOKEN }}
          # For private repositories, `publish_results` will always be set to `false`, regardless
          # of the value entered here.
          publish_results: false

      # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF
      # format to the repository Actions tab.
      - name: "Upload artifact"
        uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
        with:
          name: SARIF file
          path: results.sarif
          retention-days: 5

      # Upload the results to GitHub's code scanning dashboard.
      - name: "Upload to code-scanning"
        uses: github/codeql-action/upload-sarif@17573ee1cc1b9d061760f3a006fc4aac4f944fd5 # v2.2.4
        with:
          sarif_file: results.sarif

The last step, Upload to cosde-scanning, is failing with

Error: Unable to upload "results.sarif" as it is not valid SARIF:
- instance.runs[2].tool.driver.rules contains duplicate item
Error: Unable to upload "results.sarif" as it is not valid SARIF:
- instance.runs[2].tool.driver.rules contains duplicate item
    at validateSarifFileSchema (/home/runner/work/_actions/github/codeql-action/17573ee1cc1b9d061760f3a006fc4aac4f944fd5/lib/upload-lib.js:193:15)
    at uploadFiles (/home/runner/work/_actions/github/codeql-action/17573ee1cc1b9d061760f3a006fc4aac4f944fd5/lib/upload-lib.js:242:9)
    at Object.uploadFromActions (/home/runner/work/_actions/github/codeql-action/17573ee1cc1b9d061760f3a006fc4aac4f944fd5/lib/upload-lib.js:136:18)
    at async run (/home/runner/work/_actions/github/codeql-action/17573ee1cc1b9d061760f3a006fc4aac4f944fd5/lib/upload-sarif-action.js:48:30)
    at async runWrapper (/home/runner/work/_actions/github/codeql-action/17573ee1cc1b9d061760f3a006fc4aac4f944fd5/lib/upload-sarif-action.js:70:9)

Looking at the output printed right above, this item is appearing twice:

 {
        "id": "CodeReviewID",
        "name": "Code-Review",
        "helpUri": "https://github.com/ossf/scorecard/blob/376f465c111c39c6a5ad7408e8896cd790cb5219/docs/checks.md#code-review",
        "shortDescription": {
          "text": "Code-Review"
        },
        "fullDescription": {
          "text": "Determines if the project requires code review before pull requests (aka merge requests) are merged."
        },
        "help": {
          "text": "Determines if the project requires code review before pull requests (aka merge requests) are merged.",
          "markdown": "**Remediation (click \"Show more\" below)**:\n\n- If the project has only one contributor, or does not have enough reviewers to practically require that all contributions be reviewed, try to recruit more maintainers to the project who will be willing to review others' work. Ideally at least some of these people will be from different organizations (see [Contributors](checks.md#contributors)). If the project has very limited utility, consider expanding its intended utility so more people will be interested in improving it, and make that larger scope clear to potential contributors.\n\n- Follow security best practices by performing strict code reviews for every new pull request / merge request.\n\n- Make \"code reviews\" mandatory in your repository configuration. ([Instructions for GitHub.](https://docs.github.com/en/github/administering-a-repository/about-protected-branches#require-pull-request-reviews-before-merging))\n\n- Enforce the rule for administrators / code owners as well. ([Instructions for GitHub.](https://docs.github.com/en/github/administering-a-repository/about-protected-branches#include-administrators))\n\n\n\n**Severity**: High\n\n\n\n**Details**:\n\nRisk: `High` (unintentional vulnerabilities or possible injection of malicious\n\ncode)\n\n\n\nThis check determines whether the project requires code review before pull\n\nrequests (merge requests) are merged.\n\n\n\nReviews detect various unintentional problems, including vulnerabilities that\n\ncan be fixed immediately before they are merged, which improves the quality of\n\nthe code. Reviews may also detect or deter an attacker trying to insert\n\nmalicious code (either as a malicious contributor or as an attacker who has\n\nsubverted a contributor's account), because a reviewer might detect the\n\nsubversion.\n\n\n\nThe check first tries to detect whether [Branch-Protection](checks.md#branch-protection) is enabled on the\n\ndefault branch with at least one required reviewer. If this fails, the check\n\ndetermines whether the most recent (~30) commits have a Github-approved review\n\nor if the merger is different from the committer (implicit review). It also\n\nperforms a similar check for reviews using\n\n[Prow](https://github.com/kubernetes/test-infra/tree/master/prow#readme) (labels\n\n\"lgtm\" or \"approved\") and [Gerrit](https://www.gerritcodereview.com/) (\"Reviewed-on\" and \"Reviewed-by\").\n\n\n\nNote: Requiring reviews for all changes is infeasible for some projects, such as\n\nthose with only one active participant. Even a project with multiple active\n\ncontributors may not have enough active participation to be able to require\n\nreview of all proposed changes. Projects with a small number of active\n\nparticipants instead sometimes aim for a review of a\n\npercentage of proposals (e.g., \"at least half of all proposed changes are\n\nreviewed\").\n\n\n\nRequiring review does not eliminate all risks. The other reviewers might fail to\n\nnotice unintentional vulnerabilities or malicious code, be colluding with a\n\nmalicious developer, or even be the same person (using a \"[sock\n\npuppet](https://en.wikipedia.org/wiki/Sock_puppet_account)\" account).\n\n"
        },
        "defaultConfiguration": {
          "level": "error"
        },
        "properties": {
          "precision": "high",
          "problem.severity": "error",
          "security-severity": "7.0",
          "tags": [
            "supply-chain",
            "security",
            "source-code",
            "code-reviews"
          ]
        }
      },

Currently I have no workaround. Branch protection is enabled on our repo, and we are requiring code review with minimum one reviewer.

@faximan faximan added the kind/bug Something isn't working label Feb 8, 2024
@spencerschrock
Copy link
Contributor

spencerschrock commented Feb 8, 2024

This was fixed in v2.1.3 (latest version is v2.3.1 though!), here's the original issue for more details ossf/scorecard-action/issues/1094

Using the default scorecard.yaml

I assume you mean the starter workflow, we will likely need to send an update with a version bump.
https://github.com/actions/starter-workflows/blob/main/code-scanning/scorecard.yml

@ossf/scorecard-maintainers we need to decide if we're ok not hash pinning these starter workflows. Maybe leave a comment recommending the pinning?

@faximan
Copy link
Author

faximan commented Feb 8, 2024

Thanks so much for the quick reply, I can verify that bumping to 2.3.1 fixes this.

Sorry for not testing with the latest version, I kinda assumed the starter workflow would use that.

@faximan faximan closed this as completed Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants