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

Allow specifying commit_id #217

Open
eriksw opened this issue Jun 13, 2023 · 0 comments
Open

Allow specifying commit_id #217

eriksw opened this issue Jun 13, 2023 · 0 comments

Comments

@eriksw
Copy link

eriksw commented Jun 13, 2023

Use of this action in its current form (and examples) puts users at risk of a race condition variant of pull request hijacking.

The way such an attack would work is this: As a user who has write access to a repo but is NOT supposed to be able to merge code on my own, I wait until an automated workflow creates a PR that will be auto-approved by this action. If I can push my own malicious commit onto the PR branch in the moment between when the PR is created and this action is run to approve the PR, the malicious commit will be approved.

Fortunately, there is a straightforward way to fix this! If you allow specifying the commit_id to be approved, even if a malicious commit is pushed onto the branch before the approval action fires, the approval won't allow merging the malicious commit because the approval will be non-current.

It would be used in a workflow like this:

      - name: Create Pull Request
        id: create-pull-request
        uses: peter-evans/create-pull-request@v5
      - name: Generate Token (Approver)
        id: generate-approver-token
        uses: tibdex/github-app-token@v1
        if: ${{ steps.create-pull-request.outputs.pull-request-number }}
        with:
          app_id: ${{ secrets.APPROVER_APP_ID }}
          private_key: ${{ secrets.APPROVER_APP_PRIVATE_KEY }}
      - name: Approve Pull Request
        uses: hmarr/auto-approve-action@v3
        if: ${{ steps.generate-approver-token.outputs.token }}
        with:
          commit-id: ${{ steps.create-pull-request.outputs.pull-request-head-sha }}
          github-token: ${{ steps.generate-approver-token.outputs.token }}
          pull-request-number: ${{ steps.create-pull-request.outputs.pull-request-number }}
eriksw added a commit to retailnext/pproxy that referenced this issue Jun 13, 2023
*   Use a separate app for approving the PR

    Use curl for now:
    hmarr/auto-approve-action#217

*   Generate a nice commit message showing the changes

*   Fix broken references to outputs

*   Set PR title, author, message correctly

*
Signed-off-by: Erik Swanson <erik@retailnext.net>
eriksw added a commit to retailnext/pproxy that referenced this issue Jun 13, 2023
*   Use a separate app for approving the PR

    Use curl for now:
    hmarr/auto-approve-action#217

*   Generate a nice commit message showing the changes

*   Fix broken references to outputs

*   Set PR title, author, message correctly

*   Use environment-scoped secrets

Signed-off-by: Erik Swanson <erik@retailnext.net>
eriksw added a commit to retailnext/pproxy that referenced this issue Jun 13, 2023
*   Use a separate app for approving the PR

    Use curl for now:
    hmarr/auto-approve-action#217

*   Generate a nice commit message showing the changes

*   Fix broken references to outputs

*   Set PR title, author, message correctly

*   Use environment-scoped secrets

Signed-off-by: Erik Swanson <erik@retailnext.net>
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

No branches or pull requests

1 participant