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

Unclear error message when GitHub dependency graph is disabled #164

Closed
WillDaSilva opened this issue Jul 21, 2022 · 13 comments
Closed

Unclear error message when GitHub dependency graph is disabled #164

WillDaSilva opened this issue Jul 21, 2022 · 13 comments
Labels
bug Something isn't working

Comments

@WillDaSilva
Copy link
Contributor

When a GitHub repository is newly forked, it has the dependency graph feature disabled by default. If dependency-review-action is used on one of these freshly cloned repositories, it errors with the following message:

Error: Forbidden

The error handling code that produces this unclear message is:

  } catch (error) {
    if (error instanceof RequestError && error.status === 404) {
      core.setFailed(
        `Dependency review could not obtain dependency data for the specified owner, repository, or revision range.`
      )
    } else if (error instanceof RequestError && error.status === 403) {
      core.setFailed(
        `Dependency review is not supported on this repository. Please ensure that Dependency graph is enabled, see https://github.com/${github.context.repo.owner}/${github.context.repo.repo}/settings/security_analysis`
      )
    } else {
      if (error instanceof Error) {
        core.setFailed(error.message)
      } else {
        core.setFailed('Unexpected fatal error')
      }
    }
  }

Specifically, it's the core.setFailed(error.message) fallback that is responsible.

Printing out the error message using core.debug(JSON.stringify(error, Object.getOwnPropertyNames(error))) results in the following:

{
  stack: 'HttpError: Forbidden\n' +
    '    at /home/runner/work/_actions/WillDaSilva/dependency-review-action/main/webpack:/dependency-review-action/node_modules/@octokit/request/dist-node/index.js:86:1\n' +
    '    at processTicksAndRejections (node:internal/process/task_queues:96:5)\n' +
    '    at Job.doExecute (/home/runner/work/_actions/WillDaSilva/dependency-review-action/main/webpack:/dependency-review-action/node_modules/bottleneck/light.js:405:1)',
  message: 'Forbidden',
  name: 'HttpError',
  status: 403,
  response: { status: 403, headers: {} },
  request: { request: {}, headers: {} },
  code: 403,
  headers: {}
}

It's clear that this ought to result in the Dependency review is not supported on this repository [...] error message, but for some reason isn't.

@Miladiir
Copy link

I also get this error on one repo where security graph is enabled. Just a bunch of mysterious 403: Forbidden with no way to debug.

@febuiles febuiles added the bug Something isn't working label Jul 25, 2022
@febuiles
Copy link
Contributor

@WillDaSilva Thanks for bringing this up 🙇. Are you seeing this in public repos? private? both?

I have yet to try and reproduce, but I'm not surprised to see this given our lack of awareness of forks.

@WillDaSilva
Copy link
Contributor Author

WillDaSilva commented Jul 25, 2022

Here's an example run where it occurred: https://github.com/WillDaSilva/meltano/actions/runs/2713985191. Please ignore the debugging print statements.

That repository was/is public, and is a fork of https://github.com/meltano/meltano/

When that workflow was run the security graph was disabled. After enabling the security graph I ran the workflow again, and it behaved properly.

@tspascoal
Copy link
Contributor

@febuiles This also happens if the repo is not public and GHAS is not enabled on the repo.

@tspascoal
Copy link
Contributor

tspascoal commented Aug 1, 2022

@febuiles
This should fix it, but it seems a dirty fix.

    if ((error as RequestError)?.status === 404) {
      core.setFailed(
        `Dependency review could not obtain dependency data for the specified owner, repository, or revision range.`
      )
    } else if ((error as RequestError)?.status === 403) {
      // eslint-disable-next-line @typescript-eslint/no-explicit-any
      github
      if ((github.context as any).repository_visibility !== 'public') {
        core.setFailed(
          `Dependency review is not supported on this repository. Please ensure that GitHub Advanced Security and Dependency graph are enabled on this repository, see https://github.com/${github.context.repo.owner}/${github.context.repo.repo}/settings/security_analysis`
        )
      } else {
        core.setFailed(
          `Dependency review is not supported on this repository. Please ensure that Dependency graph is enabled, see https://github.com/${github.context.repo.owner}/${github.context.repo.repo}/settings/security_analysis`
        )
      }
    }

as @WillDaSilva mentioned the returned exception from request doesn't seem to be a RequestError, so just watching for status won't be possible due to type checking. We can force a cast, but it doesn't seem right to me.

Context also has repository_visibililty however it's not visible on the exposed context type so we need to cast to any :( (we can always get the visibility with an extra call to the API.

@febuiles
Copy link
Contributor

febuiles commented Aug 1, 2022

@tspascoal thank you for the snippet, I hope folks find it useful while we find a longer term solution.

The story behind forks is not great atm (e.g. you can enable Dependency Graph but you can't disable it, package mapping does not work 100% of the time), and I'd like to take some time to see if there's fixes that can be made over there instead of moving this logic to handle faulty cases to the action.

@Snailedlt
Copy link

Getting the same error (namely Error: Forbidden) here in a private repo with dependency graph and dependabot enabled. The repo is not a fork either.

Here's the config we use:

# Dependency Review Action
#
# This Action will scan dependency manifest files that change as part of a Pull Request, surfacing known-vulnerable versions of the packages declared or updated in the PR. Once installed, if the workflow run is marked as required, PRs introducing known-vulnerable packages will be blocked from merging.
#
# Source repository: https://github.com/actions/dependency-review-action
# Public documentation: https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/about-dependency-review#dependency-review-enforcement
name: 'Dependency Review'
on: [pull_request]

permissions:
  contents: read

jobs:
  dependency-review:
    runs-on: ubuntu-latest
    steps:
      - name: 'Checkout Repository'
        uses: actions/checkout@v3
      - name: Dependency Review
        uses: actions/dependency-review-action@v2
        with:
          # Possible values: "critical", "high", "moderate", "low"
          fail-on-severity: high
          # Complete list of configuration options:
          # https://github.com/actions/dependency-review-action#configuration-options

And here's the log with debug enabled:

##[debug]Evaluating condition for step: 'Dependency Review'
##[debug]Evaluating: success()
##[debug]Evaluating success:
##[debug]=> true
##[debug]Result: true
##[debug]Starting: Dependency Review
##[debug]Loading inputs
##[debug]Evaluating: github.token
##[debug]Evaluating Index:
##[debug]..Evaluating github:
##[debug]..=> Object
##[debug]..Evaluating String:
##[debug]..=> 'token'
##[debug]=> '***'
##[debug]Result: '***'
##[debug]Loading env
Run actions/dependency-review-action@v[2](https://github.com/fmfaDigitalisering/Selvbetjening-frontend/actions/runs/3361880880/jobs/5573836691#step:3:2)
  with:
    fail-on-severity: high
    repo-token: ***
    fail-on-scopes: runtime
Error: Forbidden
##[debug]Node Action run completed with exit code 1
##[debug]Finishing: Dependency Review

Screenshot of the log:
image

@febuiles
Copy link
Contributor

@Snailedlt is the organization where you're running this part of GitHub Advanced Security? I think that's the only requirement we have for private repos.

@Snailedlt
Copy link

@febuiles I'm not sure. How do I check that? If that's the issue, I would hope there was a better error message though

@febuiles
Copy link
Contributor

febuiles commented Nov 1, 2022

@Snailedlt Advanced Security is a paid product, if you're not sure you can talk to the organization/enterprise owner.

Another way to find out if Advanced Security is enabled for the repo is is to see if you have the rich diff enabled for manifests in private repos. Can you see a rich diff of the PR where the Action run is failing?

Should look something like

@Snailedlt
Copy link

@febuiles Thanks for the details. I'm not an organization owner, so I can't check if we have Advanced Security in an easy way.

However it seems like it's disabled, since we don't have a rich diff as far as I can tell:
image

@febuiles
Copy link
Contributor

febuiles commented Nov 2, 2022

@Snailedlt thanks for the extra details. I think we can use @tspascoal's code snippet from above to fix the, but we need to confirm if 403s can also come from invalid repo-token/external-repo-token configs.

Keeping this as an enhancement issue open until someone can contribute a PR, if not I hope we can get to this by the next major release.

@febuiles
Copy link
Contributor

I think the clarity of the error messages has been improved in #370 (thanks again @felickz!). I'm closing this issue, please re-open if this still a problem.

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

6 participants
@febuiles @Miladiir @tspascoal @WillDaSilva @Snailedlt and others