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

Prevent Unauthorized Users from creating issues in devpool #1330

Merged
merged 65 commits into from
Aug 23, 2024

Conversation

jordan-ae
Copy link
Contributor

Resolves #31

This pull request introduces enhancements preventing unauthorized bots from creating issues in the devpool-directory repository. Specifically, it implements a mechanism to detect and close issues created by entities that are not authorized, based on their association with our approved GitHub organizations.

Key Changes:

Authorization Check for Issue Creation:

  • Implemented a check to ensure that only bots associated with our approved GitHub organizations (ubiquity, ubiquibot, or UbiquityOS) can create issues.
  • Unauthorized bots are blocked from creating issues, enhancing the security and integrity of the repository.

Issue Closure for Unauthorized Attempts:

  • Attempted to delete unauthorized issues programmatically, but since GitHub doesn't provide an endpoint for this, I explored alternative solutions.
  • After trying a bash script shared by @Keyrxng , which claims to enable programmatic deletion of GitHub issues, I was unable to make it work successfully.
  • As a fallback, unauthorized issues are now closed immediately to prevent them from polluting the repository.

Workflow Testing:
Conducted a workflow QA run to test the closing of unauthorized issues:
Workflow QA Run
Closed Unauthorized Issue

To make this workflow run possible, I temporarily disabled the check for unauthorized users when creating an issue. Under normal circumstances, this check should prevent unauthorized bots from creating issues altogether.

@Keyrxng
Copy link
Member

Keyrxng commented Aug 21, 2024

@jordan-ae tests have failed

If it's not possible to delete issues via a github cli script and we must rely on closing issues then you will have to verify that those closed issues are not affecting the statistics calculations.

I don't think the state_reason matters within the devpool especially if you verify stats are not affected.

I see that all of your issues have been closed in your forked devpool-directory which is going to make life difficult for future development if all issues are immediately closed in forks. This should be handled elegantly similar to how we handle spam links by checking if it's a fork

@Keyrxng
Copy link
Member

Keyrxng commented Aug 21, 2024

I can delete issues from my devpool-directory using the GH cli no problem @jordan-ae

image

It should be possible to write a script which performs this programmatically. Flags exist to delete without confirmation so no need to handle command-line inputs.

You could fetch all issues, filter those that aren't auth'd and loop all urls deleting each with an execSync call if using TS or whatever it would be in bash

@jordan-ae
Copy link
Contributor Author

@Keyrxng i attempt to do do several times during the day unsuccessfully. I'll try again first thing tomorrow morning

@Keyrxng
Copy link
Member

Keyrxng commented Aug 21, 2024

EDIT: My mistake, I thought for a sec the original spec was a separate cron job from sync-issues

@jordan-ae
Copy link
Contributor Author

@Keyrxng I've given deleting issues a try again and still not been able to do so programmatically. The flag to delete without confirmation doesn't bypass github's restriction. Here's the authorization error that keeps popping up. I've bumped up my bot permission to have repository: read & write permissions but it still requires comfirmation

Screenshot from 2024-08-21 22-36-20

 - name: Close Unauthorized Issues
        run: |
          REPO="jordan-ae/devpool-directory"
          AUTHORIZED_ORG_IDS=(76412717 133917611 165700353)
          
          # Fetch issues with author login and author association (organization info might be absent)
          issues=$(gh issue list --repo "$REPO" --limit 100 --json number,author,title)

          # Check if issues JSON is valid
          if [[ -z "$issues" || "$issues" == "[]" ]]; then
            echo "No issues found or invalid JSON."
            exit 0
          fi
          
          # Process each issue
          echo "$issues" | jq -c '.[]' | while read -r issue; do
            issue_number=$(echo "$issue" | jq -r '.number')
            issue_author_login=$(echo "$issue" | jq -r '.author.login')
            issue_title=$(echo "$issue" | jq -r '.title')
            author_association=$(echo "$issue" | jq -r '.authorAssociation')
      
            if [[ ! " ${AUTHORIZED_ORG_IDS[@]} " =~ " ${issue_author_login} " && "$author_association" != "OWNER" ]]; then
              echo "Closing unauthorized issue: #$issue_number $issue_title (by $issue_author_login)..."
              gh issue delete "$issue_number" --repo "$REPO" --yes
            fi
          done 

@Keyrxng
Copy link
Member

Keyrxng commented Aug 22, 2024

@jordan-ae I'm not certain but it's likely your token/permissions. If that's the standard runner token that's being used it won't have admin rights to the org, you may have to pass a custom token.

I'd have went the way of writing it in TS personally as I'd find it easier to debug but that's just my preference.

I've bumped up my bot permission to have repository: read & write permissions but it still requires comfirmation

It's not your bot token that's being used I don't think it looks like it's the standard runner/repo token. Have you tried it with a PAT which has admin rights to the org where the issues exist?

@jordan-ae
Copy link
Contributor Author

jordan-ae commented Aug 22, 2024

Btw @Keyrxng & @rndquu is it okay if I add a delete unauthorized issues step to the sync-issues.yml workflow ? That will basically on workflow run get rid of any issues that haven't been created by an authorized entity without ever having to sync them in. Like in this run

@Keyrxng
Copy link
Member

Keyrxng commented Aug 22, 2024

Btw @Keyrxng & @rndquu is it okay if I add a delete unauthorized issues step to the sync-issues.yml workflow ? That will basically on workflow run get rid of any issues that haven't been created by an authorized entity without ever having to sync them in. Like in this run

I guess it can be either a step in the workflow or included in the Typescript logic. I'd prefer it built with TS but I wouldn't prevent merging for that reason.

Whatever way you choose to handle it, you need to consider the fact that other contributors after forking the devpool and running your workflow will automatically have all issues deleted which would make life difficult for future development. So this should be handled elegantly similar to how we did with spam links.

@Keyrxng
Copy link
Member

Keyrxng commented Aug 22, 2024

@rndquu can you delete the two issues created from jordans bot?

issue_title=$(echo "$issue" | jq -r '.title')
author_association=$(echo "$issue" | jq -r '.authorAssociation')

if [[ ! " ${AUTHORIZED_ORG_IDS[@]} " =~ " ${issue_author_login} " && "$author_association" != "OWNER" ]]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is only for bots. I suppose thats fine because humans shouldn't open issues here anyways.

- name: Delete Unauthorized Issues
run: bash ./scripts/delete-unauthorized-issues.sh
env:
GH_TOKEN: ${{ secrets.PAT }}
Copy link
Member

@0x4007 0x4007 Aug 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't do this with having to add an extra secret PAT when we already have a bot auth token from the previous step. Use the bot auth and I can merge this in.

      - name: Get GitHub App token
        uses: tibdex/[email protected]
        id: get_installation_token
        with:
          app_id: ${{ secrets.APP_ID }}
          private_key: ${{ secrets.APP_PRIVATE_KEY }}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also can you confirm that this script works? I'm pretty sure the default environment variable name is GITHUB_TOKEN?

@0x4007
Copy link
Member

0x4007 commented Aug 23, 2024

I am assuming that the bot auth token acts the same as a PAT so it should work but technically this is untested.

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 this pull request may close these issues.

Delete issues created by contributors
3 participants