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

Esh07 · 2023-09-21T00:05:31Z

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 [2.0] Malicious Link Detection and multiple lines changes or replacing lines in PR handle by Github Action script #74916

💡 New features:

Delete previous comments posted by the script on the PR.
Check multiple line changes in the PR (PR replacing text also flag as multiple changes) 🆕
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) 🆕
Add automatic closing on offensive content. 🆕
Drop a message to the contributor if the PR is closed due to offensive content. 🆕
Add reopened event to trigger GitHub-action. 🆕
Post comment on PR that contains multiple file changes. 🆕
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:

Replace line in Contributors.md
- test with replacing one line with new script Esh07/first-contributions#198
Multiple files changed in PR
- Test multiple files change - Contributors.md and README.md file Esh07/first-contributions#150
Only new added in Contributors.md file
- Update Contributors.md Esh07/first-contributions#207
2 Additions and 2 Deletions lines in Contributors.md file
- 2 additions and 2 deletion in Contributors.md file Esh07/first-contributions#211
3 new lines additions in Contributors.md file
- Add 3 new lines additions in Contributors.md file Esh07/first-contributions#213
Check for inappropriate use of words in PR
- doc: added offensive content -> shush Esh07/first-contributions#226

Checklist

Before submitting this PR, please make sure:

You have read and followed the contributing guidelines.
You have tested your script on a forked repository.
You have updated the documentation if needed.
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.

I have refactor the script and separate the content checking, fetching PR and modified file as separate job in runner

Additionally, the script is now more cover the use case such if user replace one line and add their line then script will accept that as well. Co-Authored-By: Roshan Jossy <8488446+Roshanjossey@users.noreply.github.com>

Implemented code to check offensive content with purgoman API. There is no implementation (commented out) for checking URLs

Merge code is been separated into new job and it will have substitute steps jobs. Added sub-job for checking mergeability

If the content contain offensive text then it will close the PR and one variable will update to true/false.

Here I break down the code to only perform single action such as merge request.

A separate job for only publishing comment with token permission to issue:write. There are 3 types of comments - comment about multiples files modified - Comment on merge conflict - Comment on multiple line changes in Contributors.md - Comment about Code of Conduct in PR which used offensive language Script also add review label to PR if there multiple file changes

@rp-1701

PR will get message once PR get merged. This code will randomly generate GIF and tag user. Thanks to - @rp-1701 - share link dev.io - @Roshanjossey - review - @rossana87 - for reddit logo - @NextThread - For share link to hackernews Co-Authored-By: Roshan Jossy <8488446+Roshanjossey@users.noreply.github.com> Co-Authored-By: Rossana Ventrella <99794555+rossana87@users.noreply.github.com> Co-Authored-By: Rohan Patil <107268275+rp-1701@users.noreply.github.com> Co-authored-by: Anurag Roy <binarybeast@Anurags-MacBook-Air.local>

Auto v3 test

There was format error 'Invalid workflow file You have an error in your yaml syntax on line 416' Script: | and underneath script was not align properly

Jobs does not have an id attribute and it is not required to mention. The job name itself is ID here is the issue I faced: **Invalid workflow file:** .github/workflows/auto-pr-merge.yml#L11 The workflow is not valid. .github/workflows/auto-pr-merge.yml (Line: 11, Col: 5): Unexpected value 'id' .github/workflows/auto-pr-merge.yml (Line: 113, Col: 5): Unexpected value 'id'

There were invalid job references via IDs. Here is the error: The workflow is not valid. .github/workflows/auto-pr-merge.yml (Line: 365, Col: 25): Unrecognized named-value: 'check-non-safe-content'. Located at position 1 within expression: check-non-safe-content.check-non-safe-content.outputs.is_safe_contents .github/workflows/auto-pr-merge.yml **Before:** 'check-non-safe-content.check-non-safe-content.output.is_safe_content' **After:** 'check-non-safe-content.output.is_safe_content' (Line: 368, Col: 24): Unrecognized named-value: 'merge-pr'. Located at position 1 within expression: merge-pr.merge-pr.outputs.is_pr_mergeable **Before:** 'merge-pr.merge-pr.is_pr_mergeable' **After:** 'merge-pr.is_pr_mergeable' The issue resolved in this commit

fixing this error The workflow is not valid. .github/workflows/auto-pr-merge.yml (Line: 365, Col: 25): Unrecognized named-value: 'check-non-safe-content'. Located at position 1 within expression: check-non-safe-content.outputs.is_safe_contents .github/workflows/auto-pr-merge.yml (Line: 368, Col: 24): Unrecognized named-value: 'merge-pr'. Located at position 1 within expression: merge-pr.outputs.is_pr_mergeable

Jobs if statement cannot utilize the env instead needs. Fix it by replacing `.env` with `.needs`

…ue or false

…ut variable

Esh07 · 2023-10-06T22:09:25Z

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 and others added 30 commits September 13, 2023 15:50

chore: refactor and content check as separate job

6e201a4

I have refactor the script and separate the content checking, fetching PR and modified file as separate job in runner

chore: refactor code for checking whether one line modified

25ae698

Additionally, the script is now more cover the use case such if user replace one line and add their line then script will accept that as well. Co-Authored-By: Roshan Jossy <8488446+Roshanjossey@users.noreply.github.com>

chore: add seperate job for offensive content check

4a4379a

Implemented code to check offensive content with purgoman API. There is no implementation (commented out) for checking URLs

chore: refactor merge code into separate job with multiple steps

53ce60d

Merge code is been separated into new job and it will have substitute steps jobs. Added sub-job for checking mergeability

chore: add close PR step in merge job

0d3ba11

If the content contain offensive text then it will close the PR and one variable will update to true/false.

chore: refactor merge step part to only merge action

5a58534

Here I break down the code to only perform single action such as merge request.

Merge pull request #145 from Esh07/auto-v3-test

5bffae9

Auto v3 test

Fix the error on the script

9fd1df6

There was format error 'Invalid workflow file You have an error in your yaml syntax on line 416' Script: | and underneath script was not align properly

chore: fix the issue of wrong usages of references in if by using needs

9692f70

Jobs if statement cannot utilize the env instead needs. Fix it by replacing `.env` with `.needs`

fix: put the == in if statement.

58d0dba

Update auto-pr-merge.yml

d523b2c

test: try to fix the issue with untest code

2374958

fix: added fromJson(PR_FILES) to covert str to json

6e5dfcc

add pr_files env to single job

74a3ee3

remove double qoute from env template

91087e3

changed from fromJSON to toJSON

1e61ae2

added dubug statements

91cda1e

Update auto-pr-merge.yml

21066f5

Update auto-pr-merge.yml

0753ee8

Update auto-pr-merge.yml

f9d4695

Update auto-pr-merge.yml

12de78c

Update auto-pr-merge.yml

e487aaa

Added fromJSON parson back to variable

806e715

Update auto-pr-merge.yml

8655e9f

Update auto-pr-merge.yml

562cde8

Esh07 added 25 commits October 2, 2023 16:17

Update Contributors.md

7cd7b17

doc: added offensive content -> shush (#214)

2675926

add dubg statements

4482541

fix the "ech" to "echo" to valid sytax

15ac5b8

doc: added offensive content -> shush (#216)

0bfb2c0

more dubg code enjoy esh

81312b3

doc: added offensive content -> shush (#217)

a0696c5

debug more, going out of hand dude

c9f6eec

no python comment code in js . dude comon

723350b

doc: added offensive content -> shush (#219)

ba28e65

more console logs

3ca89ed

more dubg in to hell

ca61bd7

Update auto-pr-merge.yml

5aa3cee

Update auto-pr-merge.yml

448be0b

doc: added offensive content -> shush (#222)

79beb8c

Update auto-pr-merge.yml

25812d7

Update auto-pr-merge.yml

2ff916b

Update auto-pr-merge.yml

c6605e1

doc: added offensive content -> shush (#223)

426cb33

chore: converts the checking multiple line changes part into javscript

b066b14

chore: replace false to var "is_one_line_changed " to make dynamic tr…

cfeaa95

…ue or false

update error status of fail PR in checking offensive content step

20db288

chore: added back the stroing lsit of changed file in in env and outp…

1e685d7

…ut variable

chore: update the logic of showing list of file names

327a492

remove the const body twice mention

e754e62

Esh07 marked this pull request as ready for review October 2, 2023 21:43

Merge branch 'main' into main

064d2fd

Esh07 mentioned this pull request Oct 6, 2023

Maintainer Request. #74564

Closed

Esh07 mentioned this pull request Oct 9, 2023

[2.0] Malicious Link Detection and multiple lines changes or replacing lines in PR handle by Github Action script #74916

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Esh07 commented Sep 21, 2023 •

edited

Esh07 commented Oct 6, 2023

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

Are you sure you want to change the base?

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

Conversation

Esh07 commented Sep 21, 2023 • edited

TL;DR

Description

Changes

Permissions

Testing

Checklist

Feedback

Esh07 commented Oct 6, 2023

Esh07 commented Sep 21, 2023 •

edited