fix: verify correct permission names based on OpenAPI schema #142

octokitbot · 2022-10-26T09:04:34Z

BREAKING CHANGE: several permissions have been renamed to match GitHub's OpenAPI spec. It's really just a fix, but in order to avoid friction we decided to release a breaking change for this one.

I found new changes on https://docs.github.com/en/rest/reference/permissions-required-for-github-apps/ and thought I'd let you know about it 👋🤖

I can't tell if the changes include fixes, features, breaking changes or just cache updates, you'll have to figure that out on yourself and adapt the commit messages accordingly to trigger the right release, see our commit message conventions.

gr2m

Something's not right, the new route should have been added. I guess it's the line break in the HTML

kfcampbell · 2022-10-26T16:16:28Z

@gr2m is this indicative of an error upstream we can report somewhere?

gr2m · 2022-10-26T19:13:28Z

@gr2m is this indicative of an error upstream we can report somewhere?

I looked into it and I don't think so. It looks like the way the HTML is rendered is independent of how the route was added in its source file.

I think we should address this case on our side.

gr2m · 2022-10-28T21:39:50Z

Okay there were bigger changes that happened here, like title changes.

I think it's time we utilze the schema now that the we have a complete schema for app-permissions in the OpenAPI spec now. We can get the names of the permissions out of that and be sure that they match, and then parse the contents of the page and assign the routes to the respective permission.

gr2m · 2022-10-28T22:14:28Z

scripts/update.js

+const MISSING_SCHEMA_PERMISSIONS = [
+  "codespaces",
+  "dependabot_secrets",
+  "email_addresses",
+  "followers",
+  "git_ssh_keys",
+  "gpg_keys",
+  "interaction_limits",
+  "organization_events",
+  "organization_webhooks",
+  "profile",
+  "repository_webhooks",
+  "self_hosted_runners",
+  "starring",
+];


@kfcampbell @nickfloyd can you please look up the real names of these permissions and also reach out to the OpenAPI team and let them know that these permissions are missing in the schema at components/schemas/app-permissions

We can ship the PR as is and remove these later, or add our own mapping until the OpenAPI spec is updated.

Hey @gr2m will do... thanks for the head's up!

Updating here: I talked to the Apps team and they're receptive of a PR internally to add these permissions.

gr2m · 2022-10-28T22:16:27Z

It looks like our parse app permissions where quite incorrect for a while.

Instead of just parsing titles form https://docs.github.com/en/rest/overview/permissions-required-for-github-apps and assuming they are the correct permission names used by GitHub, we now load the OpenAPI spec and utilize the app-permission schema component in there. I added a mapping where some titles are different from the permission names used internally, but also found several permissions that are listed in the docs but not in the schema.

generated/api.github.com.json

gr2m

I think we should get these fixed otherwise this would be a additional breaking changes.

meta is actually metadata so one might say there are breaking changes anyway, but really they are just fixes as meta has never been correct in the first place

gr2m · 2022-10-28T22:26:04Z

generated/api.github.com.json

@@ -208,13 +212,33 @@
        "POST /repos/{owner}/{repo}/check-suites/{check_suite_id}/rerequest"
      ]
    },
-    "codespaces": {


removes codespaces because it's not defined in OpenAPI schema and I'm not sure if this is the correct name

gr2m · 2022-10-28T22:26:24Z

generated/api.github.com.json

-        "DELETE /user/interaction-limits",
-        "PUT /user/interaction-limits"
-      ]
-    },


removes all the above because they are not defined in OpenAPI schema and I'm not sure if this is the correct name

gr2m · 2022-10-28T22:27:30Z

generated/api.github.com.json

-      "url": "https://docs.github.com/en/free-pro-team@latest/rest/reference/permissions-required-for-github-apps/#permission-on-keys",
-      "read": ["GET /user/keys", "GET /user/keys/{key_id}"],
-      "write": ["DELETE /user/keys/{key_id}", "POST /user/keys"]
-    },


there are likely different permissions for git_ssh_keys and gpg_keys but we need to verify or get the OpenAPI schema updated

gr2m · 2022-10-28T22:27:42Z

generated/api.github.com.json

-        "POST /orgs/{org}/hooks",
-        "POST /orgs/{org}/hooks/{hook_id}/pings"
-      ]
-    },


both missing in OpenAPI schema

gr2m · 2022-10-28T22:27:50Z

generated/api.github.com.json

-        "DELETE /orgs/{org}/blocks/{username}",
-        "PUT /orgs/{org}/blocks/{username}"
-      ]
-    },


missing in OpenAPI schema

gr2m · 2022-10-28T22:27:58Z

generated/api.github.com.json

-        "PATCH /repos/{owner}/{repo}/hooks/{hook_id}",
-        "POST /repos/{owner}/{repo}/hooks"
-      ]
-    },


missing in OpenAPI schema

gr2m · 2022-10-28T22:29:18Z

generated/api.github.com.json

-        "POST /orgs/{org}/actions/runners/{runner_id}/labels",
-        "PUT /orgs/{org}/actions/runners/{runner_id}/labels"
-      ]
-    },


security_events is added here: https://github.com/octokit/app-permissions/pull/142/files?show-viewed-files=true&file-filters%5B%5D=#diff-260b0feb344434d5b14f971a900a1ed6041cf8c0a5a393cd806a75277ce3c5c5R215

self_hosted_runners and dependabot_secrets are missing in schema

kfcampbell · 2023-10-17T18:45:17Z

This is a long-running issue that we've all probably lost context for (I know I have). @gr2m / @nickfloyd, would you both be interested in doing a zoom call sometime to sort this PR out?

gr2m · 2023-10-17T19:59:01Z

Next step is still to update the OpenAPI spec:
https://github.com/octokit/app-permissions/pull/142/files?show-viewed-files=true&file-filters%5B%5D=#r1008542042
could one of you look into that?

and look through my comments on updates to the generated/api.github.com.json file. I don't think we need a zoom call at this point?

nickfloyd

I manually looked at / verified the changes in the GitHub docs for the changes mentioned - not sure if there is any more validation need here.

gr2m

let's go 🎉

github-actions · 2023-11-01T18:47:15Z

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

build: cache

74f079f

octokitbot added the Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR label Oct 26, 2022

gr2m requested changes Oct 26, 2022

View reviewed changes

gr2m self-assigned this Oct 28, 2022

gr2m added 4 commits October 28, 2022 15:10

use app permissions schema from OpenAPI spec

4914d8d

build(cache): cache/app-permissions-schema.json

5e23c68

ci(update): run on new OpenAPI releases

6cb66b1

fix: update permissions

b4ce7c8

gr2m reviewed Oct 28, 2022

View reviewed changes

meta permission is actually called metadata

7a3d19d

gr2m changed the title ~~🤖📯 App permissions changed~~ fix: verify correct permission names based on OpenAPI scheema Oct 28, 2022

gr2m changed the title ~~fix: verify correct permission names based on OpenAPI scheema~~ fix: verify correct permission names based on OpenAPI schema Oct 28, 2022

gr2m requested changes Oct 28, 2022

View reviewed changes

generated/api.github.com.json Outdated Show resolved Hide resolved

fix

8b9b350

gr2m requested changes Oct 28, 2022

View reviewed changes

nickfloyd added the Priority: Normal label Nov 2, 2022

nickfloyd self-assigned this Nov 2, 2022

Octokit Bot added 7 commits November 29, 2022 09:06

build: cache

4e07c70

build: cache

02c8889

build: cache

8c51353

build: cache

3f3b1f9

build: cache

c0b5036

build: cache

78090b0

build: cache

3ecbb88

Octokit Bot added 14 commits July 29, 2023 09:04

build: cache

d3011b7

build: cache

263050f

build: cache

242d2a0

build: cache

070bca9

build: cache

56ddd65

build: cache

bda5422

build: cache

427db5e

build: cache

391bbe7

build: cache

2fd0c11

build: cache

558d26a

build: cache

8b28bed

build: cache

4dd5848

build: cache

6e1dcd0

build: cache

c67b8aa

Octokit Bot and others added 4 commits October 18, 2023 09:04

build: cache

36f2ce6

build: cache

b8d7d7e

Merge branch 'main' into update

f982dae

lint fixes

b09f502

nickfloyd removed the Status: Blocked Some technical or requirement is blocking the issue label Nov 1, 2023

nickfloyd approved these changes Nov 1, 2023

View reviewed changes

nickfloyd requested review from gr2m and kfcampbell November 1, 2023 18:35

gr2m reviewed Nov 1, 2023

View reviewed changes

gr2m approved these changes Nov 1, 2023

View reviewed changes

nickfloyd merged commit a91488b into main Nov 1, 2023
4 checks passed

nickfloyd deleted the update branch November 1, 2023 18:45

github-actions bot added the released label Nov 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: verify correct permission names based on OpenAPI schema #142

fix: verify correct permission names based on OpenAPI schema #142

octokitbot commented Oct 26, 2022 •

edited by nickfloyd

Loading

gr2m left a comment

kfcampbell commented Oct 26, 2022

gr2m commented Oct 26, 2022

gr2m commented Oct 28, 2022 •

edited

Loading

gr2m Oct 28, 2022

nickfloyd Nov 2, 2022

kfcampbell Nov 1, 2023

gr2m commented Oct 28, 2022

gr2m left a comment

gr2m Oct 28, 2022

gr2m Oct 28, 2022

gr2m Oct 28, 2022

gr2m Oct 28, 2022

gr2m Oct 28, 2022

gr2m Oct 28, 2022

gr2m Oct 28, 2022

kfcampbell commented Oct 17, 2023

gr2m commented Oct 17, 2023

nickfloyd left a comment

gr2m left a comment

github-actions bot commented Nov 1, 2023

fix: verify correct permission names based on OpenAPI schema #142

fix: verify correct permission names based on OpenAPI schema #142

Conversation

octokitbot commented Oct 26, 2022 • edited by nickfloyd Loading

gr2m left a comment

Choose a reason for hiding this comment

kfcampbell commented Oct 26, 2022

gr2m commented Oct 26, 2022

gr2m commented Oct 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gr2m commented Oct 28, 2022

gr2m left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kfcampbell commented Oct 17, 2023

gr2m commented Oct 17, 2023

nickfloyd left a comment

Choose a reason for hiding this comment

gr2m left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 1, 2023

octokitbot commented Oct 26, 2022 •

edited by nickfloyd

Loading

gr2m commented Oct 28, 2022 •

edited

Loading