fix(github): preserve selected repositories on org secret update (#6519)

Co-authored-by: Gergely Bräutigam <gergely.brautigam@sap.com>
Signed-off-by: Alexander Chernov <alexander@chernov.it>

providers/v1/github/client.go

@@ -34,6 +34,10 @@ import (
 
 const errWriteOnlyProvider = "not implemented - this provider supports write-only operations"
 
+// orgSecretVisibilitySelected is the GitHub org-secret visibility value that
+// restricts the secret to an explicit list of repositories.
+const orgSecretVisibilitySelected = "selected"
+
 // https://github.com/external-secrets/external-secrets/issues/644
 var _ esv1.SecretsClient = &Client{}
 
@@ -61,6 +65,9 @@ type Client struct {
 	createOrUpdateFn func(ctx context.Context, eSecret *github.EncryptedSecret) (*github.Response, error)
 	listSecretsFn    func(ctx context.Context) (*github.Secrets, *github.Response, error)
 	deleteSecretFn   func(ctx context.Context, ref esv1.PushSecretRemoteRef) (*github.Response, error)
+	// listSelectedReposFn lists the repo IDs currently granted access to a
+	// "selected"-visibility org secret; nil for repo/env scopes.
+	listSelectedReposFn func(ctx context.Context, name string) (github.SelectedRepoIDs, error)
 }
 
 // DeleteSecret deletes a secret from GitHub Actions.
@@ -136,6 +143,19 @@ func (g *Client) PushSecret(ctx context.Context, secret *corev1.Secret, remoteRe
 		Visibility:     visibility,
 	}
 
+	// A "selected"-visibility org secret restricts access to an explicit list
+	// of repositories. GitHub treats an update that omits
+	// selected_repository_ids as "clear all repositories", so without re-sending
+	// the current list an update silently revokes access for every previously
+	// selected repository. Preserve the existing associations on update.
+	if visibility == orgSecretVisibilitySelected && githubSecret != nil && g.listSelectedReposFn != nil {
+		repoIDs, err := g.listSelectedReposFn(ctx, name)
+		if err != nil {
+			return fmt.Errorf("failed to list selected repositories for org secret %q: %w", name, err)
+		}
+		encryptedSecret.SelectedRepositoryIDs = repoIDs
+	}
+
 	if _, err := g.createOrUpdateFn(ctx, encryptedSecret); err != nil {
 		return fmt.Errorf("failed to create secret: %w", err)
 	}

providers/v1/github/client_test.go

@@ -240,6 +240,68 @@ func TestPushSecret(t *testing.T) {
 	}
 }
 
+func TestPushSecretSelectedRepos(t *testing.T) {
+	validKey := withGetPublicKeyFn(&github.PublicKey{
+		Key:   new("Zm9vYmFyCg=="),
+		KeyID: new("123"),
+	}, nil, nil)
+	secret := &corev1.Secret{Data: map[string][]byte{"foo": []byte("bingg")}}
+	ref := esv1alpha1.PushSecretData{
+		Match: esv1alpha1.PushSecretMatch{SecretKey: "foo"},
+	}
+
+	t.Run("selected visibility preserves existing repositories", func(t *testing.T) {
+		var pushed *github.EncryptedSecret
+		g := Client{provider: &esv1.GithubProvider{}}
+		g.getSecretFn = withGetSecretFn(&github.Secret{Name: "foo", Visibility: "selected"}, nil, nil)
+		g.getPublicKeyFn = validKey
+		g.listSelectedReposFn = func(_ context.Context, _ string) (github.SelectedRepoIDs, error) {
+			return github.SelectedRepoIDs{1, 2, 3}, nil
+		}
+		g.createOrUpdateFn = func(_ context.Context, es *github.EncryptedSecret) (*github.Response, error) {
+			pushed = es
+			return nil, nil
+		}
+		require.NoError(t, g.PushSecret(context.TODO(), secret, ref))
+		require.NotNil(t, pushed)
+		assert.Equal(t, "selected", pushed.Visibility)
+		assert.Equal(t, github.SelectedRepoIDs{1, 2, 3}, pushed.SelectedRepositoryIDs)
+	})
+
+	t.Run("list selected repos error is propagated", func(t *testing.T) {
+		g := Client{provider: &esv1.GithubProvider{}}
+		g.getSecretFn = withGetSecretFn(&github.Secret{Name: "foo", Visibility: "selected"}, nil, nil)
+		g.getPublicKeyFn = validKey
+		g.listSelectedReposFn = func(_ context.Context, _ string) (github.SelectedRepoIDs, error) {
+			return nil, errors.New("boom")
+		}
+		g.createOrUpdateFn = withCreateOrUpdateSecretFn(nil, nil)
+		err := g.PushSecret(context.TODO(), secret, ref)
+		assert.ErrorContains(t, err, "failed to list selected repositories")
+	})
+
+	t.Run("non-selected visibility does not set repositories", func(t *testing.T) {
+		var pushed *github.EncryptedSecret
+		called := false
+		g := Client{provider: &esv1.GithubProvider{}}
+		g.getSecretFn = withGetSecretFn(&github.Secret{Name: "foo", Visibility: "all"}, nil, nil)
+		g.getPublicKeyFn = validKey
+		g.listSelectedReposFn = func(_ context.Context, _ string) (github.SelectedRepoIDs, error) {
+			called = true
+			return github.SelectedRepoIDs{9}, nil
+		}
+		g.createOrUpdateFn = func(_ context.Context, es *github.EncryptedSecret) (*github.Response, error) {
+			pushed = es
+			return nil, nil
+		}
+		require.NoError(t, g.PushSecret(context.TODO(), secret, ref))
+		require.NotNil(t, pushed)
+		assert.False(t, called)
+		assert.Equal(t, "all", pushed.Visibility)
+		assert.Nil(t, pushed.SelectedRepositoryIDs)
+	})
+}
+
 func TestResolveOrgSecretVisibility(t *testing.T) {
 	ptr := func(s string) *string { return &s }
 	tests := []struct {

providers/v1/github/org_secrets.go

@@ -43,3 +43,24 @@ func (g *Client) orgListSecretsFn(ctx context.Context) (*github.Secrets, *github
 func (g *Client) orgDeleteSecretsFn(ctx context.Context, remoteRef esv1.PushSecretRemoteRef) (*github.Response, error) {
 	return g.baseClient.DeleteOrgSecret(ctx, g.provider.Organization, remoteRef.GetRemoteKey())
 }
+
+// orgListSelectedRepoIDs returns the IDs of all repositories currently granted
+// access to a "selected"-visibility organization secret, following pagination.
+func (g *Client) orgListSelectedRepoIDs(ctx context.Context, name string) (github.SelectedRepoIDs, error) {
+	ids := github.SelectedRepoIDs{}
+	opts := &github.ListOptions{PerPage: 100}
+	for {
+		repos, resp, err := g.baseClient.ListSelectedReposForOrgSecret(ctx, g.provider.Organization, name, opts)
+		if err != nil {
+			return nil, err
+		}
+		for _, repo := range repos.Repositories {
+			ids = append(ids, repo.GetID())
+		}
+		if resp == nil || resp.NextPage == 0 {
+			break
+		}
+		opts.Page = resp.NextPage
+	}
+	return ids, nil
+}

providers/v1/github/provider.go

@@ -70,6 +70,7 @@ func newClient(ctx context.Context, store esv1.GenericStore, kube client.Client,
 	g.createOrUpdateFn = g.orgCreateOrUpdateSecret
 	g.listSecretsFn = g.orgListSecretsFn
 	g.deleteSecretFn = g.orgDeleteSecretsFn
+	g.listSelectedReposFn = g.orgListSelectedRepoIDs
 	ghClient, err := g.AuthWithPrivateKey(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("could not get private key: %w", err)
@@ -81,6 +82,8 @@ func newClient(ctx context.Context, store esv1.GenericStore, kube client.Client,
 		g.createOrUpdateFn = g.repoCreateOrUpdateSecret
 		g.listSecretsFn = g.repoListSecretsFn
 		g.deleteSecretFn = g.repoDeleteSecretsFn
+		// Repo and env secrets have no "selected repositories" concept.
+		g.listSelectedReposFn = nil
 		if provider.Environment != "" {
 			// For environment to work, we need the repository ID instead of its name.
 			repo, _, err := ghClient.Repositories.Get(ctx, g.provider.Organization, g.provider.Repository)