Feature request: precheck more permissions needed by other steps in verify step #895
Comments
Can you help me understand the rationale for this? I understand it would be bad for |
The rationale is that it wasn't necessary so far. We can use But I think we might be able to use GraphQL to check the necessary permissions, with a query like this query ($owner: String!,$repo:String!) {
repository(owner:$owner,name:$repo) {
# Any of these should suffice: ADMIN, MAINTAIN, WRITE. Maybe TRIAGE, too, but need to verify if TRIAGE is sufficient to create a release
# https://docs.github.com/en/graphql/reference/enums#repositorypermission
viewerPermission
}
}We'd need to experiment if these match the permissions we actually need |
|
Oh nice. Yeah definitely better to use that if we can |
|
Just jotting down what the next steps would be as I don't know when I'll have the time. If anyone else would like to go through these tests that'd be great! We need to verify the above query with both classic tokens and the new fine-grained tokens. And we need to test it in both a private and a public repository. We need to test if we can do the following
Classic tokens
Fine grained
|
|
Thanks! I'll look into that next week |
|
@gr2m you mean |
|
@gr2m I double checked and the Classic PAT docs say
I submitted an issue to GitHub Support (ticket 2935830) |
|
Oh...according to the docs
So it seems to get the permission of the user associated with the PAT, rather than the permission of the PAT itself, so this won't help us. I think this is equivalent to the REST route |
|
According to this there is a way to check the permissions of a classic PAT but not a fine-grained PAT. |
yes 🤣
Yes. Maybe we just start out with this, it's better than nothing, and do a follow up issue to add checks for installation access tokens / fine-grained user tokens? |
|
Has the API team scheduled a ticket for a way to query fine-grained PAT permissions for like, within a year or so? If not then I still think it would be worth implementing checks via same-value writes now, and relying on those until the day we can query the permissions. I mean, I guess I could start with a PR for classic PAT permission checks, and then make a separate one for same-value write checks |
|
either sounds good to me because either is better than what we have today :) Checking with |
|
Oh my gosh, I just realized...we can check |
|
Oh, argh. Again, |
Related to #738, it would be good to try to verify that we have as many of the necessary GitHub permissions as possible before running other steps. For example, publish can succeed but then adding issue comments can fail during the
successstep; then adding those comments manually is the only option. But if we could detect the lack of permissions and error out on theverifystep, then the user can fix their token and rerun, and the issue comments will be created successfully after publish.As I mentioned in #738:
What I've found out so far is:
git pushwith no changesMaybe we can check permission to update issues and releases by doing a no-op update on one (sending the title it already has in an update, etc), I will have to experiment. But we'd be able to avoid hacky workarounds if the GitHub API provided an explicit way to check if we have permissions to do a certain operation.
The text was updated successfully, but these errors were encountered: