Github SafeSettings invokes DELETE API on incorrect settings

[sajithvasu]

Hello,

Github Safesettings calls the DELETE api to remove all the repositories when you add incorrect config in the settings.yml file. Error seems to be bit concerning as it calls delete api. Also, the error states that it will remove the repo on Mon, 01 Feb 2021.

Sanitized logs:
←[92mxxxx-safesettings-←[0m ←[32mxxxx-safesettings←[0m [@octokit/request] "DELETE https://api.github.com/teams/5430398/repos/{org}/{repo}" is deprecated. It is scheduled to be removed on Mon, 01 Feb 2021 00:00:00 GMT. See https://developer.github.com/changes/2020-01-21-moving-the-team-api-endpoints/
←[92mxxxx-safesettings←[0m ←[32mxxxx-safesettings←[0m error calling find for Teams HttpError: You cannot remove repositories from a security manager team. for repo: {"owner":"{org}","repo":"{repo}","branch":"main","required_pull_request_reviews":{"required_approving_review_count":1},"enforce_admins":null,"restrictions":null,"headers":{"accept":"application/vnd.github.hellcat-preview+json,application/vnd.github.luke-cage-preview+json,application/vnd.github.zzzax-preview+json"}} entries [{"name":"security-reader","permission":"security-scan-reviewer"}]

settings.yml:

repository:
teams:
  - name: security-reader
    permission: security-scan-reviewer
branches:  
  - name: default
    protection:      
      required_pull_request_reviews:
        required_approving_review_count: 1
      enforce_admins: 
      restrictions:

Note: We are building the docker image from the dockerFile provided in this repo and running the app on AKS platform.

Is this a bug within the code?

[anderssonjohan]

About the deprecation message. This is because the now deprecated URL route is used for PUT/DELETE requests in teams.js:

safe-settings/lib/plugins/teams.js

Lines 3 to 5 in 15518fe

	// it is necessary to use this endpoint until GitHub Enterprise supports
	// the modern version under /orgs
	const teamRepoEndpoint = '/teams/:team_id/repos/:owner/:repo'

This can now safely be updated to use the new URL pattern made available in GHES 2.21:
https://docs.github.com/en/enterprise-server@2.21/rest/reference/teams#add-or-update-team-repository-permissions

Old still used in 2.20:
https://docs.github.com/en/enterprise-server@2.20/rest/reference/teams#add-or-update-team-repository-permissions

I don't know about the deprecation policy for this project but GHES 3.3 was just deprecated, so this is really old stuff.

I want to point out that the old endpoints still work, but it's a bit dissatisfying with the deprecating date (2021!).

EDIT: The proper way is probably to revert this commit and use the octokit provided functions instead of making the requests to the deprecated teams api: 3f63f4b#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519

[nikhil-nomula]

@anderssonjohan is this a bug with the log message. If so, do we know when this can be fixed?

[nikhil-nomula]

Hi @sajithvasu, we are running into the same log message.

Can you please let us know if you are still running into this issue? And did your repository actually get deleted

[paddyroddy]

I'm getting calls to the DELETE API with valid config. It's not actually deleting anything but concerning nonetheless. Particularly when showing others.

I'm using the GHA with settings here https://github.com/UCL-MIRSG/.github/tree/main/safe-settings.

[anderssonjohan]

Sorry for the very late reply @nikhil-nomula .
This deprecation message was about the deprecated endpoint in the GitHub API. The removal date 2021 is about the deprecated endpoints, not your (?) repository.

And did your repository actually get deleted

This code is about removing repositories from teams, not deleting repositories from GitHub.
There's no code in https://github.com/github/safe-settings that calls DELETE repo:
https://github.com/search?q=repo%3Agithub%2Fsafe-settings+delete&type=code&p=1

Also note this part in the issue description:

error calling find for Teams HttpError: You cannot remove repositories from a security manager team

I believe in that particular setup it wouldn't remove repositories from the team because the role Security Manager can't do that:
https://docs.github.com/en/organizations/managing-peoples-access-to-your-organization-with-roles/using-organization-roles#about-pre-defined-organization-roles

So if it was expected to remove repos from teams but didn't, it wasn't because of using the deprecated endpoint, but because of the security manager role / lack of permission.

[anderssonjohan]

@paddyroddy For which team does it call the DELETE repo endpoint in your case? You have defined one team in the settings here: https://github.com/UCL-MIRSG/.github/blob/0117c4298294938bc8aca65ef2020d798e7fbae3/safe-settings/settings.yaml#L39-L40
If you have added other teams, e.g. by using the web ui, then safe-settings is supposed to revoke that access since it's not in the settings file. And if someone changes the access level of a team that should have access, then safe-settings will change it back to whatever is in the settings file.

[paddyroddy]

@anderssonjohan sorry I tried to add the full logs in text, but it is too long for GitHub, so I attach the logs as a text file. There aren't any teams added in the UI. The relevant section is line 3470 which starts with

2024-12-19T08:09:22.0054917Z �[34mDEBUG�[39m (github): �[36mGitHub request: DELETE https://api.github.com/orgs/UCL-MIRSG/teams/arc/repos/UCL-MIRSG/cmic-upload-assistant-genfi2 - 204�[39m

so it doesn't seem to refer to teams as far as I can tell.

0_safe-settings-sync.txt

[anderssonjohan]

@anderssonjohan sorry I tried to add the full logs in text, but it is too long for GitHub, so I attach the logs as a text file. There aren't any teams added in the UI. The relevant section is line 3470 which starts with

2024-12-19T08:09:22.0054917Z �[34mDEBUG�[39m (github): �[36mGitHub request: DELETE https://api.github.com/orgs/UCL-MIRSG/teams/arc/repos/UCL-MIRSG/cmic-upload-assistant-genfi2 - 204�[39m

so it doesn't seem to refer to teams as far as I can tell.

0_safe-settings-sync.txt

That request goes to this endpoint:
https://docs.github.com/en/rest/teams/teams?apiVersion=2022-11-28#remove-a-repository-from-a-team--code-samples

Thus, it wants to remove the team named "arc". In the linked settings.yml you only have a team named "mirsg" so it will remove anything else.

[paddyroddy]

That request goes to this endpoint: docs.github.com/en/rest/teams/teams?apiVersion=2022-11-28#remove-a-repository-from-a-team--code-samples

Thus, it wants to remove the team named "arc". In the linked settings.yml you only have a team named "mirsg" so it will remove anything else.

Okay makes sense! I had a look, and we do have a team called arc. I think what confused me is that it hadn't been added to any of the repos. Thanks for your help.

[anderssonjohan]

@paddyroddy According to the response from the GitHub API in the log you provided the team 'arc' is added to the all the listed repos.

Here's the result from the GET + the diff from your log, just before all the DELETE requests.
It seems you need to add team arc with triage permission to get rid of the unwanted deletes.

2024-12-19T08:09:21.2666080Z �[34mDEBUG�[39m (github): �[36mGitHub request: GET https://api.github.com/repos/UCL-MIRSG/terraform-gha-runner-harvester/teams - 200�[39m
2024-12-19T08:09:21.2667113Z �[34mDEBUG�[39m (probot): �[36m [
2024-12-19T08:09:21.2667521Z   {
2024-12-19T08:09:21.2667820Z     "name": "ARC",
2024-12-19T08:09:21.2668149Z     "id": 10741035,
2024-12-19T08:09:21.2668525Z     "node_id": "T_kwDOBHGq_c4Ao-Ur",
2024-12-19T08:09:21.2668933Z     "slug": "arc",
2024-12-19T08:09:21.2669337Z     "description": "All ARC staff",
2024-12-19T08:09:21.2669747Z     "privacy": "closed",
2024-12-19T08:09:21.2670212Z     "notification_setting": "notifications_disabled",
2024-12-19T08:09:21.2671337Z     "url": "https://api.github.com/organizations/74558205/team/10741035",
2024-12-19T08:09:21.2671988Z     "html_url": "https://github.com/orgs/UCL-MIRSG/teams/arc",
2024-12-19T08:09:21.2672767Z     "members_url": "https://api.github.com/organizations/74558205/team/10741035/members{/member}",
2024-12-19T08:09:21.2673649Z     "repositories_url": "https://api.github.com/organizations/74558205/team/10741035/repos",
2024-12-19T08:09:21.2674248Z     "permission": "triage",
2024-12-19T08:09:21.2674621Z     "permissions": {
2024-12-19T08:09:21.2674961Z       "admin": false,
2024-12-19T08:09:21.2675294Z       "maintain": false,
2024-12-19T08:09:21.2675642Z       "push": false,
2024-12-19T08:09:21.2675933Z       "triage": true,
2024-12-19T08:09:21.2676195Z       "pull": true
2024-12-19T08:09:21.2676449Z     },
2024-12-19T08:09:21.2676728Z     "parent": null
2024-12-19T08:09:21.2676982Z   },
2024-12-19T08:09:21.2677211Z   {
2024-12-19T08:09:21.2677444Z     "name": "MIRSG",
2024-12-19T08:09:21.2677711Z     "id": 6853909,
2024-12-19T08:09:21.2678003Z     "node_id": "T_kwDOBHGq_c4AaJUV",
2024-12-19T08:09:21.2678592Z     "slug": "mirsg",
2024-12-19T08:09:21.2678964Z     "description": "All ARC MIRSG RTPs",
2024-12-19T08:09:21.2679305Z     "privacy": "closed",
2024-12-19T08:09:21.2679654Z     "notification_setting": "notifications_enabled",
2024-12-19T08:09:21.2680152Z     "url": "https://api.github.com/organizations/74558205/team/6853909",
2024-12-19T08:09:21.2680672Z     "html_url": "https://github.com/orgs/UCL-MIRSG/teams/mirsg",
2024-12-19T08:09:21.2681294Z     "members_url": "https://api.github.com/organizations/74558205/team/6853909/members{/member}",
2024-12-19T08:09:21.2681998Z     "repositories_url": "https://api.github.com/organizations/74558205/team/6853909/repos",
2024-12-19T08:09:21.2682659Z     "permission": "admin",
2024-12-19T08:09:21.2682963Z     "permissions": {
2024-12-19T08:09:21.2683233Z       "admin": true,
2024-12-19T08:09:21.2683497Z       "maintain": true,
2024-12-19T08:09:21.2683772Z       "push": true,
2024-12-19T08:09:21.2684037Z       "triage": true,
2024-12-19T08:09:21.2684304Z       "pull": true
2024-12-19T08:09:21.2684555Z     },
2024-12-19T08:09:21.2684793Z     "parent": null
2024-12-19T08:09:21.2685043Z   }
2024-12-19T08:09:21.2685278Z ] 
2024-12-19T08:09:21.2685406Z 
2024-12-19T08:09:21.2685513Z  [
2024-12-19T08:09:21.2685740Z   {
2024-12-19T08:09:21.2685973Z     "name": "mirsg",
2024-12-19T08:09:21.2686247Z     "permission": "admin"
2024-12-19T08:09:21.2686527Z   }
2024-12-19T08:09:21.2686803Z ] �[39m
2024-12-19T08:09:21.2693256Z �[34mDEBUG�[39m (probot): �[36mResults of comparing Teams diffable target [{"name":"ARC","id":10741035,"node_id":"T_kwDOBHGq_c4Ao-Ur","slug":"arc","description":"All ARC staff","privacy":"closed","notification_setting":"notifications_disabled","url":"https://api.github.com/organizations/74558205/team/10741035","html_url":"https://github.com/orgs/UCL-MIRSG/teams/arc","members_url":"https://api.github.com/organizations/74558205/team/10741035/members{/member}","repositories_url":"https://api.github.com/organizations/74558205/team/10741035/repos","permission":"triage","permissions":{"admin":false,"maintain":false,"push":false,"triage":true,"pull":true},"parent":null},{"name":"MIRSG","id":6853909,"node_id":"T_kwDOBHGq_c4AaJUV","slug":"mirsg","description":"All ARC MIRSG RTPs","privacy":"closed","notification_setting":"notifications_enabled","url":"https://api.github.com/organizations/74558205/team/6853909","html_url":"https://github.com/orgs/UCL-MIRSG/teams/mirsg","members_url":"https://api.github.com/organizations/74558205/team/6853909/members{/member}","repositories_url":"https://api.github.com/organizations/74558205/team/6853909/repos","permission":"admin","permissions":{"admin":true,"maintain":true,"push":true,"triage":true,"pull":true},"parent":null}] with source [{"name":"mirsg","permission":"admin"}] is [object Object]�[39m

Note: The list of teams returned by the endpoint should be the same as when using the GitHub UI. The summary above the list in the UI should say something like:

DIRECT ACCESS
X users have access to this repository.
Y members.
Z outside collaborators.
2 teams.

...where the 2 teams are arc and mirsg.

[paddyroddy]

It seems you need to add team arc with triage permission to get rid of the unwanted deletes.

Hmm... It seems like we already have that setting (but definitely not done via Safe-Settings).