From e5e44038381002bdd39e8fb85d6913016e21548e Mon Sep 17 00:00:00 2001 From: jferrl Date: Fri, 30 Oct 2026 19:25:57 +0100 Subject: [PATCH] feat: let --permissions name any permission GitHub has --permissions accepted only the names InstallationPermissions has a field for, so a permission GitHub added since, such as installation_repositories, could not be requested from the CLI. Every name now goes through InstallationPermissions.Other. The list was a guard against GitHub ignoring a name it does not know and minting a token with every permission. Tested against a real App, it does not: an unknown name gets a 422, even beside valid ones, and the CLI exits 2 with GitHub's message. AGENTS.md now points exit code 5 at --owner too. --- AGENTS.md | 2 +- cmd/githubauth/main.go | 29 +++++------------------------ cmd/githubauth/main_test.go | 14 +++++--------- 3 files changed, 11 insertions(+), 34 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d2d88d3..08ce2f8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -70,7 +70,7 @@ Branch on these rather than on message text. Each one implies a different move: | 2 | the invocation is wrong | fix the flags, do not retry unchanged | | 3 | GitHub refused the key or the App's permissions | fix the credential, do not retry unchanged | | 4 | rate limited | wait `retry_after_seconds`, then repeat | -| 5 | the App is not installed where it was asked to be | fix `--installation` | +| 5 | the App is not installed where it was asked to be | fix `--installation` or `--owner` | Under `--exec` the exit code is the one the command exited with. diff --git a/cmd/githubauth/main.go b/cmd/githubauth/main.go index 3da53ac..28f6a75 100644 --- a/cmd/githubauth/main.go +++ b/cmd/githubauth/main.go @@ -534,13 +534,10 @@ func splitRepos(v string) []string { } // parsePermissions reads --permissions into GitHub's permission object, or nil -// when it names none. A name GitHub would not recognize is reported here, -// rather than as a 422 that does not say which one. -// -// shortcut: only the names InstallationPermissions has fields for are known, so a -// newer GitHub permission is refused until the struct gains it. Unknown names -// could go through Other once GitHub is known to refuse a name it does not -// recognize, rather than ignore it and mint a token with every permission. +// when it names none. Every name goes through Other, so a permission GitHub +// adds works before the library has a field for it. GitHub answers a name it +// does not know with a 422, even beside valid ones, rather than ignoring it and +// minting a token with every permission. func parsePermissions(v string) (*githubauth.InstallationPermissions, error) { pairs := splitRepos(v) if len(pairs) == 0 { @@ -555,23 +552,7 @@ func parsePermissions(v string) (*githubauth.InstallationPermissions, error) { } levels[name] = level } - - // A map of strings always marshals, and always decodes into the struct. - raw, _ := json.Marshal(levels) - var perms githubauth.InstallationPermissions - _ = json.Unmarshal(raw, &perms) - - // Unmarshal drops unknown names silently, so read back what it kept. - raw, _ = json.Marshal(perms) - var known map[string]string - _ = json.Unmarshal(raw, &known) - for _, pair := range pairs { - name, _, _ := strings.Cut(pair, ":") - if name = strings.TrimSpace(name); known[name] == "" { - return nil, usagef("--permissions: unknown permission %q; use GitHub's name, e.g. pull_requests", name) - } - } - return &perms, nil + return &githubauth.InstallationPermissions{Other: levels}, nil } // envInt64 ignores a malformed value here, before the flags are parsed; diff --git a/cmd/githubauth/main_test.go b/cmd/githubauth/main_test.go index 1aee3b1..cfc9e2d 100644 --- a/cmd/githubauth/main_test.go +++ b/cmd/githubauth/main_test.go @@ -506,15 +506,6 @@ func TestRun(t *testing.T) { wantStdout: isEmpty, wantStderr: `"contents" is not name:level`, }, - { - name: "an unknown permission is named", - args: func(string) []string { - return []string{"token", "--client-id", "Iv1.abc", "--key", keyPath, "--installation", "9", "--permissions", "contents:read,pull-requests:write"} - }, - wantCode: exitUsage, - wantStdout: isEmpty, - wantStderr: `unknown permission "pull-requests"`, - }, { name: "exec without a command is misuse", args: func(string) []string { @@ -762,6 +753,11 @@ func TestTokenRequests(t *testing.T) { args: []string{"--repos", "api", "--permissions", "contents:read, pull_requests:write"}, wantBody: `{"repositories":["api"],"permissions":{"contents":"read","pull_requests":"write"}}`, }, + { + name: "a permission newer than the library still reaches GitHub", + args: []string{"--permissions", "installation_repositories:write"}, + wantBody: `{"permissions":{"installation_repositories":"write"}}`, + }, { name: "exec revokes the token when the command exits", args: []string{"--exec", "--", os.Args[0], "-test.run=^TestExecHelper$"},