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

Approval-based example allows publishing by anybody #267

Closed
kettanaito opened this issue Oct 27, 2024 · 8 comments · Fixed by #269
Closed

Approval-based example allows publishing by anybody #267

kettanaito opened this issue Oct 27, 2024 · 8 comments · Fixed by #269

Comments

@kettanaito
Copy link
Contributor

Hi! Thanks for an awesome tool.

A quick question: does the approval-based publishing kick in if anybody approves a pull request? This is probably a question more to GitHub Actions, but I thought you'd know.

My concern is that I want to automatically publish the package only if a team member approved it. So that random user couldn't approve potentially malicious changes.

@kettanaito
Copy link
Contributor Author

I can confirm that any approval of a PR triggers preview publishing (see mswjs/msw#2335, the approval from @dandelionadia).

This isn't an issue of this library, but a better setup would be appreciated in the README. The one listed is not secure. Anybody can trigger the preview publish, which defies the whole purpose of locking it behind the approval state.

@kettanaito kettanaito changed the title How does APPROVED state work? Approval-based example allows publishing by anybody Oct 27, 2024
@kettanaito
Copy link
Contributor Author

I've looked everywhere, but there doesn't seem to be a way to check the PR reviewer's permissions in the if expression. The user object doesn't have anything permissions-related. Looks like you must check those permissions manually, which means run and then if on individual steps, which is extremely annoying.

@kettanaito
Copy link
Contributor Author

Solution

After hours of research, this is finally the approach that worked for me:

name: release

on:
  pull_request_review:
    types: [submitted]
  workflow_dispatch:

jobs:
  check:
    # Trigger the permissions check whenever someone approves a pull request.
    # They must have the write permissions to the repo in order to
    # trigger preview package publishing.
    if: github.event.review.state == 'approved'
    runs-on: ubuntu-latest
    outputs:
      has-permissions: ${{ steps.checkPermissions.outputs.require-result }}
    steps:
      - name: Check permissions
        id: checkPermissions
        uses: actions-cool/check-user-permission@v2
        with:
          require: 'write'

  preview:
    # The approving user must pass the permissions check
    # to trigger the preview publish.
    needs: check
    if: needs.check.outputs.has-permissions == 'true'
    runs-on: macos-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v4

      # Prepare your package here...

      - name: Publish preview
        run: # RELEASE COMMAND HERE

This has two jobs so preview can be skipped entirely if check output is 'false'. This is so far the only approach I found that:

  • Doesn't mark either jobs as failed because then your entire PRs status will be failed;
  • Correctly checks for the approver's permissions (requires write);
  • Actually works.

@AmirSa12
Copy link
Member

AmirSa12 commented Oct 27, 2024

Vite has a similar process to check the permissions.
This might help: vitejs/vite#17461

you can also use github.event.comment.author_association == 'MEMBER' with comments. this way only the members of the repo can trigger the publish.

@Aslemammad
Copy link
Member

@AmirSa12 Can we add what Artem and you are suggesting to the recipes, i love both of the solutions!

@kettanaito
Copy link
Contributor Author

@AmirSa12, nice one if you base your publishing around comments. I really like the approval-based publishing, Vite's setup is a bit too complex for my liking.

@Aslemammad, I'm more than happy to open a PR if you find these approaches suitable! I believe the one featured in the README right now doesn't quite do what it's supposed to do.

@Aslemammad
Copy link
Member

💯 feel free to send a pr! excited to see it!

@kettanaito
Copy link
Contributor Author

Opened a pull request at #269. Let me know what you think.

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.

3 participants