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

Need to specify ref and sha when running a pull request from a fork with pull_request_target #51

Open
jmservera opened this issue Oct 30, 2023 · 6 comments

Comments

@jmservera
Copy link

I'm using this action in combination with the dependency review action, to check for new dependencies, including transient ones, when someone creates a PR. As it needs write access, the only way to obtain it when running from a fork is to use the pull_request_target and then change the head during checkout.
But, if I run the action with pull_request_target it is automatically sent to the main instead of the PR head.

I see that with the command-line it is possible to specify the head, is it possible to add this field to the action?

This is my workflow:

name: 50-Dependency-Review
on:
# dependency scan works comparing pull requests to the base branch
  pull_request_target:
    # These types are all required for CRDA to scan pull requests correctly and securely.
    types: [ opened, synchronize, reopened, labeled, edited ]
    paths-ignore:
      - '**/*.md'
      - '**/*.txt'

permissions:
  contents: write

jobs:
  dependency-review:
    runs-on: ubuntu-latest
    steps:
      - name: Get User Permission
        id: checkAccess
        uses: actions-cool/check-user-permission@v2
        with:
          require: write
          username: ${{ github.triggering_actor }}
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
      - name: Check User Permission
        if: steps.checkAccess.outputs.require-result == 'false'
        run: |
          echo "${{ github.triggering_actor }} does not have permissions on this repo."
          echo "Current permission level is ${{ steps.checkAccess.outputs.user-permission }}"
          echo "Job originally triggered by ${{ github.actor }}"
          exit 1
      - name: 'Checkout Repository'
        uses: actions/checkout@v4
        with:
          ref: ${{  github.event.pull_request.head.sha }} # This is dangerous without the first access check
      - run: echo "${{ github.event_name }}"
      # Use the dependency snapshot to detect transient dependencies
      # https://github.com/actions/dependency-review-action/issues/595#event-10791333872
      - name: Submit Dependency Snapshot
        uses: advanced-security/maven-dependency-submission-action@v3
        with:
          directory: ${{ github.workspace }}/todo
      - name: 'Dependency Review from PR'
        uses: actions/dependency-review-action@v3
@jmservera
Copy link
Author

I've reviewed the library being used and it specifically disallows pull_request_target, so, is there any other way to achieve what I'm trying to do? Just setting the permissions with pull_request type does not work in the case of a fork.

@jmservera jmservera changed the title Need to specify the head when running a pull request from a fork with pull_request_target Need to specify ref and sha when running a pull request from a fork with pull_request_target Oct 31, 2023
@jmservera
Copy link
Author

I finally created a modified version of the action to make it work with forks:

name: 50-Dependency-Review
on:
# dependency scan works comparing pull requests to the base branch
  pull_request_target:
  #pull_request:
    # These types are all required for CRDA to scan pull requests correctly and securely.
    types: [ opened, synchronize, reopened, labeled, edited ]
    paths-ignore:
      - '**/*.md'
      - '**/*.txt'

permissions:
  contents: write

jobs:
  dependency-review:
    runs-on: ubuntu-latest
    permissions:
      contents: write # for actions/checkout to fetch code
      security-events: write # for github/codeql-action/upload-sarif to upload SARIF results
      actions: read # only required for a private repository by github/codeql-action/upload-sarif to get the Action run status    
    steps:
      - name: Get User Permission
        id: checkAccess
        uses: actions-cool/check-user-permission@v2
        with:
          require: write
          username: ${{ github.triggering_actor }}
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
      - name: Show the two contexts
        run: |
          echo "ctxt: ${{ github.sha }} pr: ${{ github.event.pull_request.head.sha }}"
          echo "ref: /ref/pull/${{ github.event.pull_request.number }}"
      - name: Check User Permission
        if: steps.checkAccess.outputs.require-result == 'false'
        run: |
          echo "${{ github.triggering_actor }} does not have permissions on this repo."
          echo "Current permission level is ${{ steps.checkAccess.outputs.user-permission }}"
          echo "Job originally triggered by ${{ github.actor }}"
          exit 1
      - name: 'Checkout Repository'
        uses: actions/checkout@v4
        with:
          ref: ${{  github.event.pull_request.head.sha }} # This is dangerous without the first access check
      - run: echo "${{ github.event_name }}"
      # Use the dependency snapshot to detect transient dependencies
      # https://github.com/actions/dependency-review-action/issues/595#event-10791333872
      - name: Submit Dependency Snapshot
        #uses: advanced-security/maven-dependency-submission-action@v3
        uses: jmservera/maven-dependency-submission-action@extend-with-sha
        with:
          directory: ${{ github.workspace }}/todo
          sha: ${{ github.event.pull_request.head.sha }}
          ref: "refs/pull/${{ github.event.pull_request.number }}/merge"
          
      - name: 'Dependency Review from PR'
        uses: actions/dependency-review-action@v3
     

Any thoughts?

@peter-murray
Copy link
Contributor

I have provided you a branch that has the ability to set a ref and sha here, please verify that this meets your requirements https://github.com/advanced-security/maven-dependency-submission-action/tree/submit-context

@jmservera
Copy link
Author

I have provided you a branch that has the ability to set a ref and sha here, please verify that this meets your requirements https://github.com/advanced-security/maven-dependency-submission-action/tree/submit-context

I tried to fix it myself in a completely different way, but your solution looks awesome and cleaner. I'll test it right now. Thanks!

@jmservera
Copy link
Author

jmservera commented Oct 31, 2023

@peter-murray I've identified a small issue with the variable names, but it looks like it will do the trick. Thanks

@peter-murray
Copy link
Contributor

I have merged the changes to main and am now working on a v4 release that will provide this functionality

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 a pull request may close this issue.

2 participants