Skip to content

Run Danger on pull requests to danger/danger GitHub repo#1382

Closed
mathroule wants to merge 8 commits into
masterfrom
run-danger-on-ci-for-pull-request
Closed

Run Danger on pull requests to danger/danger GitHub repo#1382
mathroule wants to merge 8 commits into
masterfrom
run-danger-on-ci-for-pull-request

Conversation

@mathroule

@mathroule mathroule commented Sep 3, 2022

Copy link
Copy Markdown
Member

Run Danger on pull requests to danger/danger GitHub repository.

@mathroule mathroule changed the title Run Danger on pull request Run Danger on pull request to danger repo Sep 3, 2022
@mathroule mathroule force-pushed the run-danger-on-ci-for-pull-request branch 2 times, most recently from 3e96d06 to d61fe98 Compare September 3, 2022 09:14
Comment thread .github/workflows/CI.yml Outdated
TOKEN+='79960c12a1e067f2ec'
DANGER_GITHUB_API_TOKEN=$TOKEN RUNNING_IN_ACTIONS=true echo 'bundle exec danger --verbose'
env:
DANGER_GITHUB_API_TOKEN: 7469b4e94ce21b43e3ab7a79960c12a1e067f2ec # FIXME Store it in GitHub Actions secrets

@mathroule mathroule Sep 3, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orta do you think a new GitHub API token can be generated and added as GitHub Actions secret for https://github.com/danger-public?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Afraid not, needs to be in the code like above, secrets wont work here because it only runs on branch PRs and not forks

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you probably broke the old token by putting it in a single string too, will need to find the password for that account

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The token is not authorized anymore, if my change deauthorized the token sorry about that.
Indeed storing it as GitHub Actions secrets, will make it not available for fork PRs, but I'm not sure splitting the token in 2 parts, is the best workaround to use in terms of security.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, GitHub advises solving the problem differently: https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows.

I'll test that in this PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately using pull_request_target or workflow_run triggers will not work out of the box. Indeed, the workflow will be executed in the context of the base branch, not of the head branch / PR context.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using env var this way works, but it will not work for forked pull requests:

env:
  DANGER_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}

@mathroule mathroule changed the title Run Danger on pull request to danger repo Run Danger on pull request to danger/danger GitHub repo Sep 3, 2022
@mathroule mathroule requested a review from orta September 3, 2022 09:22
@mathroule mathroule force-pushed the run-danger-on-ci-for-pull-request branch from 5e74c2b to 8e36bbb Compare September 3, 2022 13:59
@danger danger deleted a comment from github-actions Bot Sep 3, 2022
@mathroule mathroule force-pushed the run-danger-on-ci-for-pull-request branch from 64d57e8 to 87b0310 Compare September 3, 2022 14:09
@mathroule mathroule changed the title Run Danger on pull request to danger/danger GitHub repo Run Danger on pull requests to danger/danger GitHub repo Sep 3, 2022
@mathroule mathroule force-pushed the run-danger-on-ci-for-pull-request branch from 8f9ed85 to 88d16c8 Compare September 19, 2023 09:07
Comment thread .github/workflows/CI.yml
TOKEN+='79960c12a1e067f2ec'
DANGER_GITHUB_API_TOKEN=$TOKEN RUNNING_IN_ACTIONS=true echo 'bundle exec danger --verbose'
env:
DANGER_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

secrets cannot be accessed by forked PRs, so this change isn't doing what you want it to do I think

@mathroule mathroule closed this Sep 19, 2023
@mathroule mathroule deleted the run-danger-on-ci-for-pull-request branch September 19, 2023 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants