danger-js
danger-js copied to clipboard
Recommend using `pull_request_target` on GitHub Actions
Describe the bug Fixes the issues with secrets: https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/
Mainly need to update docs, and two maybe do a check if the action is pull_request
and the PR is from a forked repo and tell folks.
This could work, but it is complex.
Danger JS relies on the API to get all its info, like the diff etc. Which means it can handle the different head on your CI, but it would be very surprising to fs.readFileSync
and get the old version of the file.
This realistically shouldn't be recommended I think
@orta is there no other way to use dangerjs for projects that follow a forking workflow, via GitHub Actions? I tried browsing the docs and the relevant issues and the only approaches are:
- use
pull_request_target
: potentially risky and some functionalities won't work because the files checked out would not be from fork` - use a GitHub token (directly exposing its value) in the action: basically giving away the token, just that the token only has public repo rights
-
Both approaches don't seem to be very secure...
I think according to the advisor https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ we could potentially explore a way to split the workflow into two portions:
- (Read only) test and identify the issues or comments, do not post the comments yet. Save them in artifact
- (With write access) Retrieve the artifacts and post the comments accordingly.
So something like this:
name: Danger Checks Fork PR
# read-only
# no access to secrets
on:
pull_request:
branches:
- main
jobs:
build:
name: Danger Checks Fork PR
run : yarn danger ci --check
// save stuff into artifacts
name: Danger Write Comments for Fork PR
on:
workflow_run:
workflows: ["Danger Checks Fork PR"]
types: [completed]
jobs:
on-success:
if: ${{ github.event.workflow_run.conclusion == 'success' }}
// read stuff from artifacts
name: Danger Write
run: yarn danger ci --write
secrets:
token: ${{ secrets.GITHUB_TOKEN}}
This will require Danger to have
- a way to only do the check and export the action items
- a way to only read action items and process them
Similar things have been done for another common task facing the same challenge: creating a PR preview of the updated websites from a fork.
Do you know if the above is already available or if the whole thing is achievable with Danger?
Thanks a lot! 🙏
Yep this should be possible.
You'd probably have to write some code to this repo to handle the separation of results and processing, but it should be possible - Danger already splits those responsibilities between separate CLI commands (so that danger swift/rust/etc can run) so it could be possible with some digging right now too
Yep this should be possible.
You'd probably have to write some code to this repo to handle the separation of results and processing, but it should be possible - Danger already splits those responsibilities between separate CLI commands (so that danger swift/rust/etc can run) so it could be possible with some digging right now too
hmm, thank you! Will take a stab at it when I find the time.
Yeah, I used this approach after asking at #1373.
For reference, tho I didn't use danger at last, but I hope danger can support it natively sort of like this. like danger check action and danger comment action Benchmark (check) file: https://github.com/discordeno/discordeno/blob/main/.github/workflows/benchmark.yml Comment file: https://github.com/discordeno/discordeno/blob/main/.github/workflows/commentBenchResult.yml