first-contributions icon indicating copy to clipboard operation
first-contributions copied to clipboard

Enhanced Auto-PR: Add profanity filter, assign review tag to PRs with multiple file changes and more...

Open Esh07 opened this issue 1 year ago β€’ 2 comments

TL;DR

This PR adds an action that merges PRs that only change Contributors.md, after checking some criteria and giving feedback. It also improves the security of the token permissions by assigning them based on the steps.

  • fix https://github.com/firstcontributions/first-contributions/issues/74916

πŸ’‘ New features:

  • [x] Delete previous comments posted by the script on the PR.
  • [x] Check multiple line changes in the PR (PR replacing text also flag as multiple changes) πŸ†•
  • [x] Implemented text check (profanity filter) but not link check (Google Safe Browsing APIs is not effectively compare to VirusTotal APIs but there is limitation of making request in day on VirustTotal. To be discuss further) πŸ†•
  • [x] Add automatic closing on offensive content. πŸ†•
  • [x] Drop a message to the contributor if the PR is closed due to offensive content. πŸ†•
  • [x] Add reopened event to trigger GitHub-action. πŸ†•
  • [x] Post comment on PR that contains multiple file changes. πŸ†•
  • [x] Add review πŸ‘©πŸΌβ€πŸ’» Waiting for review to PR which contains multiple file changes. πŸ†•

πŸ†• The script will mark the PR with a β€œ Requested Changes” comment if there is action needed from user side. This will indicate that the PR is still pending.

Description

This PR adds a GitHub action that will automatically merge pull requests that only modify the Contributors.md file. The action will check if the pull request only changes one line in the file, and if the added content is safe and respectful. The action will also post comments on the pull request to provide feedback and guidance to the contributors.

Changes

This PR includes the following changes from the previous version of the script:

πŸ“ˆ The script now uses output and env variables to share job results, eliminating redundant requests and enhancing efficiency. This is an improvement from the previous version, which made redundant requests in each step.

  • Token permissions are assigned based on step requirements, enhancing security by limiting write access to necessary steps only, such as merging pull requests, and defaulting to read access otherwise.
  • Delete any previous message by github-actions bot.
  • Add review label to PR if changes multiple files.
  • Added a new job for checking offensive and malicious added content, using an external service and a regular expression.
  • Added a step job to close PR on offensive content.
  • Added some logic for retrying the request for checking the mergeability of the pull request, using a while loop and a counter:
    • GitHub does have internal checks going on when new PR created . A good discussion here and also some time it takes time to update status of pull request.
    • Script uses JS promise to make 3 attempt (with pause of 3 sec interval) if mergeability value is null.
    • Replace normal comment to review comment such as 'Request Change" comment with links to follow to solve the merge conflict.
  • The script now specifically retrieves the additions, deletions, and changes properties from the Contributors.md file. And not overall one. (Previous script checking overall additions, deletions and changes done in PR).
  • The comment messages have been enhanced and the formatting has been improved using markdown.

Permissions

🚨 Do not check out, build, or run untrusted code from the pull request withpull_request_target event. Use pull_request instead for these purposes. Good example by GitGuardian video

This PR sets minimal permissions for each job and step, to ensure that they only have access to what they need. For example:

  • checking all modified files and content: The job for checking all modified files and content only has read permissions for contents and pull requests, and write permissions for issues.

    • This is because it needs to read the contents of the repository and pull requests to check the changes, and it may need to comment on the PR (which requires issues: write permission) if multiple files or lines were changed.
  • checking offensive and malicious: The job for checking offensive and malicious links has read permissions for contents and issues to write.

    • Write permissions for issues are necessary to close them and post comments.
  • merging PRs: The job for merging PRs has write permissions for contents, pull requests and issues.

    • This is because it needs to modify the contents of the repository (which requires contents: write permission) and merge pull requests (which requires pull requests: write permission).
    • Posting a congratulatory message upon successful merges requires issues: write permission.

The steps that use GitHub scripts only have access to the GitHub token with the necessary scopes. These permissions limit the exposure of sensitive data (console log of GITHUB_TOKEN) and prevent unauthorised actions on the repository.

Testing

These are test cases I performed:

  1. Replace line in Contributors.md
    • https://github.com/Esh07/first-contributions/pull/198
  2. Multiple files changed in PR
    • https://github.com/Esh07/first-contributions/pull/150
  3. Only new added in Contributors.md file
    • https://github.com/Esh07/first-contributions/pull/207
  4. 2 Additions and 2 Deletions lines in Contributors.md file
    • https://github.com/Esh07/first-contributions/pull/211
  5. 3 new lines additions in Contributors.md file
    • https://github.com/Esh07/first-contributions/pull/213
  6. Check for inappropriate use of words in PR
    • https://github.com/Esh07/first-contributions/pull/226

Checklist

Before submitting this PR, please make sure:

  • [x] You have read and followed the contributing guidelines.
  • [x] You have tested your script on a forked repository.
  • [x] You have updated the documentation if needed.
  • [x] You have resolved any merge conflicts with the main branch. - need to resolve conflict on Contributors.md file.

Feedback

Thank you for taking the time to read this lo-o-o-o-o-o-o-ng PR description πŸ˜„. I appreciate your patience.

Please feel free to share your suggestions and improvements. I value your feedback and I want to know how we can improve it further.

Thank you again for your support and encouragement.

Esh07 avatar Sep 21 '23 00:09 Esh07

Thank you for your pull request. This pull request contains changes in files which requires review. The following files were changed:

  • .github/workflows/auto-pr-merge.yml
  • Contributors.md

github-actions[bot] avatar Sep 21 '23 00:09 github-actions[bot]

Hi @Roshanjossey, this version of the script leaves a β€˜Requested change’ comment when it detects replaced lines or multiple changes. Do you think we should also close PRs when such changes are detected?

I’d appreciate your thoughts on this.

Esh07 avatar Oct 06 '23 22:10 Esh07