From 502b64582d8b040112beed3d33aaa7e2e2025294 Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 3 Jul 2025 16:53:35 +0200 Subject: [PATCH] Support --no-repos-selected on secret set --- .../secret-org-with-selected-visibility.txtar | 44 ++++++ acceptance/testdata/secret/secret-org.txtar | 2 + pkg/cmd/secret/set/set.go | 28 +++- pkg/cmd/secret/set/set_test.go | 136 ++++++++++++------ 4 files changed, 167 insertions(+), 43 deletions(-) create mode 100644 acceptance/testdata/secret/secret-org-with-selected-visibility.txtar diff --git a/acceptance/testdata/secret/secret-org-with-selected-visibility.txtar b/acceptance/testdata/secret/secret-org-with-selected-visibility.txtar new file mode 100644 index 000000000..9d6bfed84 --- /dev/null +++ b/acceptance/testdata/secret/secret-org-with-selected-visibility.txtar @@ -0,0 +1,44 @@ +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} +env2upper SECRET_NAME=${SCRIPT_NAME}_${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Confirm organization secret does not exist, will fail admin:org scope missing +exec gh secret list --org ${ORG} +! stdout ${SECRET_NAME} + +# Set an organization secret with no shared visibility, but no repos +exec gh secret set ${SECRET_NAME} --org ${ORG} --body 'just an organization secret' --no-repos-selected + +# Defer organization secret cleanup +defer gh secret delete ${SECRET_NAME} --org ${ORG} + +# Verify new organization secret exists with shared visibility +exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME} --jq '.visibility' +stdout selected + +# Verify the secret is not shared with any repositories +exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME}/repositories --jq '.repositories | length' +stdout 0 + +# Set the same organization secret with shared visibility to the previously created repository +exec gh secret set ${SECRET_NAME} --org ${ORG} --body 'just an organization secret' --repos ${REPO} + +# Verify the secret is now shared with the repository +exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME}/repositories --jq '.repositories[0].name' +stdout ${REPO} + +# Set the same organization secret with shared visibility back to no repositories selected +exec gh secret set ${SECRET_NAME} --org ${ORG} --body 'just an organization secret' --no-repos-selected + +# Verify the secret is not shared with any repositories +exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME}/repositories --jq '.repositories | length' +stdout 0 diff --git a/acceptance/testdata/secret/secret-org.txtar b/acceptance/testdata/secret/secret-org.txtar index 7d383009c..3465628b7 100644 --- a/acceptance/testdata/secret/secret-org.txtar +++ b/acceptance/testdata/secret/secret-org.txtar @@ -1,4 +1,6 @@ # Setup environment variables used for testscript +# This script will most likely fail because you are most likely targeting a repo that is not public and an org +# that is not on the right plan: https://docs.github.com/en/actions/how-tos/security-for-github-actions/security-guides/using-secrets-in-github-actions#creating-secrets-for-an-organization env REPO=${SCRIPT_NAME}-${RANDOM_STRING} env2upper SECRET_NAME=${SCRIPT_NAME}_${RANDOM_STRING} diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index 0a6581559..7fbc77552 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -53,6 +53,11 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command Prompter: f.Prompter, } + // It is possible for a user to say `--no-repos-selected=false --repos cli/cli` and that would be equivalent to not + // specifying the flag at all. We could avoid this by checking whether the flag was set at all, but it seems like + // more trouble than it's worth since anyone who does `--no-repos-selected=false` is gonna get what's coming to them. + var noRepositoriesSelected bool + cmd := &cobra.Command{ Use: "set ", Short: "Create or update secrets", @@ -90,6 +95,9 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command # Set organization-level secret visible to specific repositories $ gh secret set MYSECRET --org myOrg --repos repo1,repo2,repo3 + # Set organization-level secret visible to no repositories + $ gh secret set MYSECRET --org myOrg --no-repos-selected + # Set user-level secret for Codespaces $ gh secret set MYSECRET --user @@ -131,6 +139,14 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command return err } + if err := cmdutil.MutuallyExclusive("specify only one of `--repos` or `--no-repos-selected`", len(opts.RepositoryNames) > 0, noRepositoriesSelected); err != nil { + return err + } + + if err := cmdutil.MutuallyExclusive("`--no-repos-selected` must be omitted when used with `--user`", opts.UserSecrets, noRepositoriesSelected); err != nil { + return err + } + if len(args) == 0 { if !opts.DoNotStore && opts.EnvFile == "" { return cmdutil.FlagErrorf("must pass name argument") @@ -148,11 +164,16 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command return cmdutil.FlagErrorf("`--repos` is only supported with `--visibility=selected`") } - if opts.Visibility == shared.Selected && len(opts.RepositoryNames) == 0 { - return cmdutil.FlagErrorf("`--repos` list required with `--visibility=selected`") + if opts.Visibility != shared.Selected && noRepositoriesSelected { + return cmdutil.FlagErrorf("`--no-repos-selected` is only supported with `--visibility=selected`") } + + if opts.Visibility == shared.Selected && (len(opts.RepositoryNames) == 0 && !noRepositoriesSelected) { + return cmdutil.FlagErrorf("`--repos` or `--no-repos-selected` required with `--visibility=selected`") + } + } else { - if len(opts.RepositoryNames) > 0 { + if len(opts.RepositoryNames) > 0 || noRepositoriesSelected { opts.Visibility = shared.Selected } } @@ -170,6 +191,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command cmd.Flags().BoolVarP(&opts.UserSecrets, "user", "u", false, "Set a secret for your user") cmdutil.StringEnumFlag(cmd, &opts.Visibility, "visibility", "v", shared.Private, []string{shared.All, shared.Private, shared.Selected}, "Set visibility for an organization secret") cmd.Flags().StringSliceVarP(&opts.RepositoryNames, "repos", "r", []string{}, "List of `repositories` that can access an organization or user secret") + cmd.Flags().BoolVar(&noRepositoriesSelected, "no-repos-selected", false, "No repositories can access the organization secret") cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "The value for the secret (reads from standard input if not specified)") cmd.Flags().BoolVar(&opts.DoNotStore, "no-store", false, "Print the encrypted, base64-encoded value instead of storing it on GitHub") cmd.Flags().StringVarP(&opts.EnvFile, "env-file", "f", "", "Load secret names and values from a dotenv-formatted `file`") diff --git a/pkg/cmd/secret/set/set_test.go b/pkg/cmd/secret/set/set_test.go index 0b305eda6..38c0fb5a9 100644 --- a/pkg/cmd/secret/set/set_test.go +++ b/pkg/cmd/secret/set/set_test.go @@ -27,88 +27,120 @@ import ( func TestNewCmdSet(t *testing.T) { tests := []struct { - name string - cli string - wants SetOptions - stdinTTY bool - wantsErr bool + name string + args string + wants SetOptions + stdinTTY bool + wantsErr bool + wantsErrMessage string }{ { name: "invalid visibility", - cli: "cool_secret --org coolOrg -v'mistyVeil'", + args: "cool_secret --org coolOrg -v mistyVeil", wantsErr: true, }, { - name: "invalid visibility", - cli: "cool_secret --org coolOrg -v'selected'", + name: "when visibility is selected, requires indication of repos", + args: "cool_secret --org coolOrg -v selected", + wantsErr: true, + wantsErrMessage: "`--repos` or `--no-repos-selected` required with `--visibility=selected`", + }, + { + name: "visibilities other than selected do not accept --repos", + args: "cool_secret --org coolOrg -v private -r coolRepo", + wantsErr: true, + wantsErrMessage: "`--repos` is only supported with `--visibility=selected`", + }, + { + name: "visibilities other than selected do not accept --no-repos-selected", + args: "cool_secret --org coolOrg -v private --no-repos-selected", + wantsErr: true, + wantsErrMessage: "`--no-repos-selected` is only supported with `--visibility=selected`", + }, + { + name: "--repos and --no-repos-selected are mutually exclusive", + args: `--repos coolRepo --no-repos-selected cool_secret`, + wantsErr: true, + wantsErrMessage: "specify only one of `--repos` or `--no-repos-selected`", + }, + { + name: "secret name is required", + args: "", wantsErr: true, }, { - name: "repos with wrong vis", - cli: "cool_secret --org coolOrg -v'private' -rcoolRepo", + name: "multiple positional arguments are not allowed", + args: "cool_secret good_secret", wantsErr: true, }, { - name: "no name", - cli: "", + name: "visibility is only allowed with --org", + args: "cool_secret -v all", wantsErr: true, }, { - name: "multiple names", - cli: "cool_secret good_secret", - wantsErr: true, - }, - { - name: "visibility without org", - cli: "cool_secret -vall", - wantsErr: true, - }, - { - name: "repos without vis", - cli: "cool_secret -bs --org coolOrg -rcoolRepo", + name: "providing --repos without --visibility implies selected visibility", + args: "cool_secret --body secret-body --org coolOrg --repos coolRepo", wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Selected, RepositoryNames: []string{"coolRepo"}, - Body: "s", + Body: "secret-body", + OrgName: "coolOrg", + }, + }, + { + name: "providing --no-repos-selected without --visibility implies selected visibility", + args: "cool_secret --body secret-body --org coolOrg --no-repos-selected", + wants: SetOptions{ + SecretName: "cool_secret", + Visibility: shared.Selected, + RepositoryNames: []string{}, + Body: "secret-body", OrgName: "coolOrg", }, }, { name: "org with selected repo", - cli: "-ocoolOrg -bs -vselected -rcoolRepo cool_secret", + args: "-o coolOrg --body secret-body -v selected -r coolRepo cool_secret", wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Selected, RepositoryNames: []string{"coolRepo"}, - Body: "s", + Body: "secret-body", OrgName: "coolOrg", }, }, { name: "org with selected repos", - cli: `--org=coolOrg -bs -vselected -r="coolRepo,radRepo,goodRepo" cool_secret`, + args: `--org coolOrg --body secret-body -v selected --repos "coolRepo,radRepo,goodRepo" cool_secret`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Selected, RepositoryNames: []string{"coolRepo", "goodRepo", "radRepo"}, - Body: "s", + Body: "secret-body", OrgName: "coolOrg", }, }, { name: "user with selected repos", - cli: `-u -bs -r"monalisa/coolRepo,cli/cli,github/hub" cool_secret`, + args: `-u --body secret-body -r "monalisa/coolRepo,cli/cli,github/hub" cool_secret`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Selected, RepositoryNames: []string{"monalisa/coolRepo", "cli/cli", "github/hub"}, - Body: "s", + Body: "secret-body", }, }, + { + name: "--user is mutually exclusive with --no-repos-selected", + args: `-u --no-repos-selected cool_secret`, + wantsErr: true, + wantsErrMessage: "`--no-repos-selected` must be omitted when used with `--user`", + }, { name: "repo", - cli: `cool_secret -b"a secret"`, + args: `cool_secret --body "a secret"`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Private, @@ -118,7 +150,7 @@ func TestNewCmdSet(t *testing.T) { }, { name: "env", - cli: `cool_secret -b"a secret" -eRelease`, + args: `cool_secret --body "a secret" --env Release`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Private, @@ -129,7 +161,7 @@ func TestNewCmdSet(t *testing.T) { }, { name: "vis all", - cli: `cool_secret --org coolOrg -b"cool" -vall`, + args: `cool_secret --org coolOrg --body "cool" --visibility all`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.All, @@ -139,7 +171,7 @@ func TestNewCmdSet(t *testing.T) { }, { name: "no store", - cli: `cool_secret --no-store`, + args: `cool_secret --no-store`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Private, @@ -148,7 +180,7 @@ func TestNewCmdSet(t *testing.T) { }, { name: "Dependabot repo", - cli: `cool_secret -b"a secret" --app Dependabot`, + args: `cool_secret --body "a secret" --app Dependabot`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Private, @@ -159,19 +191,19 @@ func TestNewCmdSet(t *testing.T) { }, { name: "Dependabot org", - cli: "-ocoolOrg -bs -vselected -rcoolRepo cool_secret -aDependabot", + args: "--org coolOrg --body secret-body --visibility selected --repos coolRepo cool_secret --app Dependabot", wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Selected, RepositoryNames: []string{"coolRepo"}, - Body: "s", + Body: "secret-body", OrgName: "coolOrg", Application: "Dependabot", }, }, { name: "Codespaces org", - cli: `random_secret -ocoolOrg -b"random value" -vselected -r"coolRepo,cli/cli" -aCodespaces`, + args: `random_secret --org coolOrg --body "random value" --visibility selected --repos "coolRepo,cli/cli" --app Codespaces`, wants: SetOptions{ SecretName: "random_secret", Visibility: shared.Selected, @@ -192,7 +224,7 @@ func TestNewCmdSet(t *testing.T) { ios.SetStdinTTY(tt.stdinTTY) - argv, err := shlex.Split(tt.cli) + argv, err := shlex.Split(tt.args) assert.NoError(t, err) var gotOpts *SetOptions @@ -208,6 +240,9 @@ func TestNewCmdSet(t *testing.T) { _, err = cmd.ExecuteC() if tt.wantsErr { assert.Error(t, err) + if tt.wantsErrMessage != "" { + assert.EqualError(t, err, tt.wantsErrMessage) + } return } assert.NoError(t, err) @@ -497,6 +532,16 @@ func Test_setRun_org(t *testing.T) { wantRepositories: []int64{1, 2}, wantApp: "actions", }, + { + name: "no repos visibility", + opts: &SetOptions{ + OrgName: "UmbrellaCorporation", + Visibility: shared.Selected, + RepositoryNames: []string{}, + }, + wantRepositories: []int64{}, + wantApp: "actions", + }, { name: "Dependabot", opts: &SetOptions{ @@ -517,6 +562,17 @@ func Test_setRun_org(t *testing.T) { wantDependabotRepositories: []string{"1", "2"}, wantApp: "dependabot", }, + { + name: "Dependabot no repos visibility", + opts: &SetOptions{ + OrgName: "UmbrellaCorporation", + Visibility: shared.Selected, + Application: shared.Dependabot, + RepositoryNames: []string{}, + }, + wantRepositories: []int64{}, + wantApp: "dependabot", + }, } for _, tt := range tests {