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

[BUG] Danger succeeds despite receiving a 403 error from GitHub API and having --failOnErrors flag set #1441

Open
tylermilner opened this issue Apr 27, 2024 · 0 comments
Labels

Comments

@tylermilner
Copy link

Describe the bug
I'm working to integrate Danger into the GitHub Actions CI one of my open source repos (see this PR). I know I don't have my GitHub PAT set quite right, but the Danger step is still succeeding despite receiving a 403 response and having the --failOnErrors flag set. See the following output of the Danger step in my workflow:

Run npm run ci-danger
  npm run ci-danger
  shell: /usr/bin/bash -e {0}
  env:
    DANGER_GITHUB_API_TOKEN: ***

> last-successful-commit-hash-action@1.0.3 ci-danger
> npx danger ci --verbose --failOnErrors


Found only messages, passing those to review.
Request failed [403]: https://api.github.com/repos/tylermilner/last-successful-commit-hash-action/issues/30/comments
Response: {
  "message": "Resource not accessible by personal access token",
  "documentation_url": "https://docs.github.com/rest/issues/comments#create-an-issue-comment"
}
Feedback: undefined
Could not add a commit status, the GitHub token for Danger does not have access rights.
If the build fails, then danger will use a failing exit code.
Danger: ✓ passed, found only messages.
## Messages
Changed Files in this PR: 
 - .github/workflows/ci.yml- package-lock.json- package.json

See screenshot below showing overall success of the workflow run despite Danger receiving the 403 error and failing to post the message to the PR.

To Reproduce
Steps to reproduce the behavior:

  1. Add Danger as a dependency of a basic JavaScript repo.
  2. Add a basic dangerfile.js to the repo (see mine below).
  3. Setup npm run script in package.json that will run Danger.
  4. Setup a basic GitHub Actions workflow that sets up Node.js, installs the project dependencies, and then runs Danger with the failOnErrors flag set.
  5. Setup a GitHub PAT on a "bot" account that doesn't have sufficient permissions to comment on a PR. In my case, I set up a fine-grained PAT with "All Repositories" access configured with "Metadata" read-only access and "Pull requests" read and write access. I realize that setting up a "classic" GitHub PATs might be a fix for this, but in this case we are intentionally trying to setup the PAT with insufficient permissions so that the 403 error is triggered.
  6. Setup DANGER_GITHUB_API_TOKEN repository secret for GitHub Actions with the value of the GitHub PAT.
  7. Trigger a workflow run via PR. Observe that the run completes successfully despite the output of the Danger step indicating a 403 error occurred and no message being posted to the PR by Danger.

Expected behavior
Danger should fail since it received a 403 error and wasn't able to post the message to the PR successful, which should ultimately result in my workflow run failing.

Screenshots
image

Your Environment

software version
danger.js 12.1.0
node 20.6.0
npm 9.8.1
Operating System Ubuntu 22.04.4 LTS

Additional context
For reference, here is the super basic dangerfile.js that I created based on the Danger "Getting Started" documentation:

import { message, danger } from 'danger'

const modifiedMD = danger.git.modified_files.join('- ')
message(`Changed Files in this PR: \n - ${modifiedMD}`)

And here is what my CI step looks like that runs Danger:

- name: Danger
  id: npm-danger
  run: npm run ci-danger
  env:
    DANGER_GITHUB_API_TOKEN: ${{ secrets.DANGER_GITHUB_API_TOKEN }}

My main concern here is that I plan to setup my GitHub PAT to expire after 1 year, at which point it would be nice for my CI builds to start failing at the "Danger" step so that I can be reminded to go back and regenerate my PAT. As it stands now, the step will silently fail which will negate any value provided by having Danger setup in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants
@tylermilner and others