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

Adding a license in 'allow-dependencies-licenses' does not prevent it from being populated in "invalid-license-changes" #764

Open
sreya opened this issue May 2, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@sreya
Copy link

sreya commented May 2, 2024

My expectation is that adding a license to allow-dependencies-licenses should prevent it from being populated in the invalid-license-changes output however that doesn't seem to be the case with the action below. Is this a bug or are my expectations for the output incorrect? If I misunderstood, how do I manually approve unlicensed packages so that they do not fail the job

  dependency-license-review:
    runs-on: ubuntu-latest
    if: github.ref != 'refs/heads/main'
    steps:
      - name: "Checkout Repository"
        uses: actions/checkout@v4
      - name: "Dependency Review"
        id: review
        # TODO: Replace this with the latest release once https://github.com/actions/dependency-review-action/pull/761 is merged.
        uses: actions/dependency-review-action@49fbbe0acb033b7824f26d00b005d7d598d76301
        with:
          allow-licenses: Apache-2.0, BSD-2-Clause, BSD-3-Clause, CC0-1.0, ISC, MIT, MIT-0, MPL-2.0
          allow-dependencies-licenses: "pkg:golang/github.com/pelletier/go-toml/v2, pkg:golang/gopkg.in/DataDog/[email protected]"
          license-check: true
          vulnerability-check: false
      - name: "Report"
        # make sure this step runs even if the previous failed
        if: always()
        shell: bash
        env:
          VULNERABLE_CHANGES: ${{ steps.review.outputs.invalid-license-changes }}
        run: |
          fields=( "unlicensed" "unresolved" "forbidden" )
          for field in "${fields[@]}"; do
            # Use jq to check if the array is not empty
            if [[ $(echo "$VULNERABLE_CHANGES" | jq ".${field} | length") -ne 0 ]]; then
              echo "$VULNERABLE_CHANGES" | jq
              exit 1
            fi
          done
          echo "No incompatible licenses detected"
{
  "unlicensed": [
    {
      "change_type": "added",
      "manifest": "go.mod",
      "ecosystem": "gomod",
      "name": "gopkg.in/DataDog/dd-trace-go.v1",
      "version": "1.63.1",
      "package_url": "pkg:golang/gopkg.in/DataDog/[email protected]",
      "license": null,
      "source_repository_url": null,
      "scope": "runtime",
      "vulnerabilities": []
    }
  ],
  "unresolved": [],
  "forbidden": []
}
@juxtin
Copy link
Contributor

juxtin commented May 2, 2024

Hi @sreya, thanks for the report. At first glance, it looks like the problem is that some of these package-urls technically violate the spec so they aren't being parsed as one might expect.

According to the purl-spec, the "name" segment must be a percent-encoded string. In your case, that means you should have:

  • pkg:golang/github.com/pelletier%2go-toml%2fv2
  • pkg:golang/gopkg.in/DataDog%[email protected]

where %2f is the percent encoding of /.

I think this will probably fix your issue. At the same time, I agree that this is kind of an annoying aspect of the purl spec, so I've also created #765 to make our parser a little more permissive.

@sreya
Copy link
Author

sreya commented May 10, 2024

Hi @juxtin I'm still receiving an error when updating to your commit 🤔 . I even tried url encoding to no avail...here's the output/workflow file:

  dependency-license-review:
    runs-on: ubuntu-latest
    if: github.ref != 'refs/heads/main'
    steps:
      - name: "Checkout Repository"
        uses: actions/checkout@v4
      - name: "Dependency Review"
        id: review
        # TODO: Replace this with the latest release once https://github.com/actions/dependency-review-action/pull/761 is merged.
        uses: actions/dependency-review-action@82ab8f69c78827a746628706b5d2c3f87231fd4c
        with:
          allow-licenses: Apache-2.0, BSD-2-Clause, BSD-3-Clause, CC0-1.0, ISC, MIT, MIT-0, MPL-2.0
          allow-dependencies-licenses: "pkg:golang/github.com/pelletier/go-toml/v2,pkg:golang/gopkg.in/DataDog%[email protected]"
          license-check: true
          vulnerability-check: false
      - name: "Report"
        # make sure this step runs even if the previous failed
        if: always()
        shell: bash
        env:
          VULNERABLE_CHANGES: ${{ steps.review.outputs.invalid-license-changes }}
        run: |
          fields=( "unlicensed" "unresolved" "forbidden" )
          for field in "${fields[@]}"; do
            # Use jq to check if the array is not empty
            if [[ $(echo "$VULNERABLE_CHANGES" | jq ".${field} | length") -ne 0 ]]; then
              echo "$VULNERABLE_CHANGES" | jq
              exit 1
            fi
          done
          echo "No incompatible licenses detected"
{
  "unlicensed": [
    {
      "change_type": "added",
      "manifest": "go.mod",
      "ecosystem": "gomod",
      "name": "gopkg.in/DataDog/dd-trace-go.v1",
      "version": "1.63.1",
      "package_url": "pkg:golang/gopkg.in/DataDog/[email protected]",
      "license": null,
      "source_repository_url": null,
      "scope": "runtime",
      "vulnerabilities": []
    }
  ],
  "unresolved": [],
  "forbidden": []
}

@spikecurtis
Copy link

Poking through the code, it looks like exclusions are processed by parsing the "package_url":

const changeAsPackageURL = parsePURL(encodeURI(change.package_url))

In the above case, the package URL from gomod is "pkg:golang/gopkg.in/DataDog/[email protected]" which does not have the slashes escaped. So, shouldn't the list of packages we want to ignore follow the same format (even if it's ambiguous vs the spec)?

Either way, I can't convince it to ignore packages, escaping the / or not.

@jonjanego jonjanego added the bug Something isn't working label May 22, 2024
@sreya
Copy link
Author

sreya commented Jun 4, 2024

Hi @juxtin any update here? Mainly wondering if there's a workaround we can use

@juxtin
Copy link
Contributor

juxtin commented Jun 14, 2024

Apologies everyone, it looks like we still have a discrepancy somewhere in how we represent or parse Go purls somewhere. We'll have to dig into this a bit more to narrow that down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants
@jonjanego @juxtin @sreya @spikecurtis and others