feat(appconfig): support private cross-owner appconfig remotes

appconfig/appconfig.go

@@ -27,6 +27,7 @@ import (
 	"strings"
 
 	"github.com/google/go-github/v90/github"
+	"github.com/palantir/go-githubapp/githubapp"
 	"github.com/rs/zerolog"
 )
 
@@ -81,6 +82,9 @@ type Loader struct {
 	parser       RemoteRefParser
 	defaultRepo  string
 	defaultPaths []string
+
+	clientCreator githubapp.ClientCreator
+	installations githubapp.InstallationsService
 }
 
 // NewLoader creates a Loader that loads configuration from paths.
@@ -139,7 +143,7 @@ func (ld *Loader) LoadConfig(ctx context.Context, client *github.Client, owner,
 			}
 			if remote != nil {
 				logger.Debug().Msgf("Found remote configuration at %s in %s", p, c.Source)
-				return ld.loadRemoteConfig(ctx, client, *remote, c)
+				return ld.loadRemoteConfig(ctx, client, owner, *remote, c)
 			}
 		}
 
@@ -159,14 +163,15 @@ func (ld *Loader) LoadConfig(ctx context.Context, client *github.Client, owner,
 	return Config{}, nil
 }
 
-func (ld *Loader) loadRemoteConfig(ctx context.Context, client *github.Client, remote RemoteRef, c Config) (Config, error) {
+func (ld *Loader) loadRemoteConfig(ctx context.Context, client *github.Client, sourceOwner string, remote RemoteRef, c Config) (Config, error) {
 	logger := zerolog.Ctx(ctx)
 	notFoundErr := fmt.Errorf("invalid remote reference: file does not exist")
 
 	owner, repo, err := remote.SplitRemote()
 	if err != nil {
 		return c, err
 	}
+	client = ld.remoteClient(ctx, client, sourceOwner, owner, repo)
 
 	path := remote.Path
 	if path == "" && len(ld.paths) > 0 {
@@ -250,7 +255,7 @@ func (ld *Loader) loadDefaultConfig(ctx context.Context, client *github.Client,
 			}
 			if remote != nil {
 				logger.Debug().Msgf("Found remote default configuration at %s in %s", p, c.Source)
-				return ld.loadRemoteConfig(ctx, client, *remote, c)
+				return ld.loadRemoteConfig(ctx, client, owner, *remote, c)
 			}
 		}
 
@@ -264,6 +269,30 @@ func (ld *Loader) loadDefaultConfig(ctx context.Context, client *github.Client,
 	return Config{}, nil
 }
 
+// remoteClient returns an installation-scoped client for remoteOwner/remoteRepo when
+// private remotes are enabled and the remote belongs to a different owner. If
+// the app is not installed on that repository, or a client cannot be created, it
+// falls back to the caller's client so public repositories continue to work.
+func (ld *Loader) remoteClient(ctx context.Context, client *github.Client, sourceOwner, remoteOwner, remoteRepo string) *github.Client {
+	if strings.EqualFold(sourceOwner, remoteOwner) || ld.clientCreator == nil || ld.installations == nil {
+		return client
+	}
+
+	logger := zerolog.Ctx(ctx)
+	installation, err := ld.installations.GetByRepository(ctx, remoteOwner, remoteRepo)
+	if err != nil {
+		logger.Warn().Err(err).Msgf("Failed to find GitHub App installation for remote configuration repository %q; using the original client", remoteOwner+"/"+remoteRepo)
+		return client
+	}
+
+	remoteClient, err := ld.clientCreator.NewInstallationClient(installation.ID)
+	if err != nil {
+		logger.Warn().Err(err).Msgf("Failed to create GitHub App client for remote configuration repository %q; using the original client", remoteOwner+"/"+remoteRepo)
+		return client
+	}
+	return remoteClient
+}
+
 // getFileContents returns the content of the file at path on ref in owner/repo
 // if it exists. Returns an empty slice and false if the file does not exist.
 func getFileContents(ctx context.Context, client *github.Client, owner, repo, ref, path string) ([]byte, bool, error) {

appconfig/appconfig_test.go

@@ -17,18 +17,55 @@ package appconfig
 import (
 	"bytes"
 	"context"
+	"errors"
 	"net/http"
 	"path/filepath"
 	"testing"
 
 	"github.com/google/go-github/v90/github"
+	"github.com/palantir/go-githubapp/githubapp"
 )
 
 const (
 	TestOwner = "test"
 	TestRef   = "develop"
 )
 
+type testInstallationsService struct {
+	githubapp.InstallationsService
+
+	installation githubapp.Installation
+	err          error
+	calls        int
+	owner        string
+	repo         string
+}
+
+func (s *testInstallationsService) GetByRepository(_ context.Context, owner, repo string) (githubapp.Installation, error) {
+	s.calls++
+	s.owner = owner
+	s.repo = repo
+	if s.err != nil {
+		return githubapp.Installation{}, s.err
+	}
+	return s.installation, nil
+}
+
+type testClientCreator struct {
+	githubapp.ClientCreator
+
+	client         *github.Client
+	err            error
+	calls          int
+	installationID int64
+}
+
+func (c *testClientCreator) NewInstallationClient(installationID int64) (*github.Client, error) {
+	c.calls++
+	c.installationID = installationID
+	return c.client, c.err
+}
+
 func TestLoadConfig(t *testing.T) {
 	tests := map[string]struct {
 		Paths    []string
@@ -142,6 +179,91 @@ func TestLoadConfig(t *testing.T) {
 	}
 }
 
+func TestLoadConfigWithPrivateRemotes(t *testing.T) {
+	newClient := func(rp *ResponsePlayer) *github.Client {
+		client, _ := github.NewClient(github.WithHTTPClient(&http.Client{Transport: rp}))
+		return client
+	}
+	localRules := func(includeRemote bool) *ResponsePlayer {
+		rp := &ResponsePlayer{}
+		rp.AddRule(ExactPathMatcher("/repos/test/remote-ref/contents/.github/test-app.yml"), filepath.Join("testdata", "remote-ref-contents.yml"))
+		if includeRemote {
+			rp.AddRule(ExactPathMatcher("/repos/remote/config/contents/config/test-app.yml"), filepath.Join("testdata", "config-contents.yml"))
+		}
+		return rp
+	}
+
+	t.Run("uses remote repository installation client", func(t *testing.T) {
+		localRules := localRules(false)
+		remoteRules := &ResponsePlayer{}
+		remoteRule := remoteRules.AddRule(ExactPathMatcher("/repos/remote/config/contents/config/test-app.yml"), filepath.Join("testdata", "config-contents.yml"))
+		installations := &testInstallationsService{installation: githubapp.Installation{ID: 42, Owner: "remote"}}
+		creator := &testClientCreator{client: newClient(remoteRules)}
+
+		loader := NewLoader([]string{".github/test-app.yml"}, WithPrivateRemotes(creator, installations))
+		cfg, err := loader.LoadConfig(context.Background(), newClient(localRules), TestOwner, "remote-ref", TestRef)
+		if err != nil {
+			t.Fatalf("unexpected error loading config: %v", err)
+		}
+		if !bytes.Equal(cfg.Content, []byte("message: hello\n")) {
+			t.Errorf("incorrect content: %s", cfg.Content)
+		}
+		if installations.calls != 1 || creator.calls != 1 || remoteRule.Count != 1 {
+			t.Errorf("expected one installation lookup, client creation, and remote request; got %d, %d, and %d", installations.calls, creator.calls, remoteRule.Count)
+		}
+		if installations.owner != "remote" || installations.repo != "config" {
+			t.Errorf("expected installation lookup for remote/config, got %s/%s", installations.owner, installations.repo)
+		}
+		if creator.installationID != 42 {
+			t.Errorf("expected installation client for ID 42, got %d", creator.installationID)
+		}
+	})
+
+	for name, test := range map[string]struct {
+		installationsErr error
+		creatorErr       error
+	}{
+		"falls back when remote repository has no installation": {installationsErr: githubapp.InstallationNotFound("remote/config")},
+		"falls back when installation client creation fails":    {creatorErr: errors.New("client creation failed")},
+	} {
+		t.Run(name, func(t *testing.T) {
+			localRules := localRules(true)
+			installations := &testInstallationsService{installation: githubapp.Installation{ID: 42, Owner: "remote"}, err: test.installationsErr}
+			creator := &testClientCreator{err: test.creatorErr}
+
+			loader := NewLoader([]string{".github/test-app.yml"}, WithPrivateRemotes(creator, installations))
+			cfg, err := loader.LoadConfig(context.Background(), newClient(localRules), TestOwner, "remote-ref", TestRef)
+			if err != nil {
+				t.Fatalf("unexpected error loading config: %v", err)
+			}
+			if !bytes.Equal(cfg.Content, []byte("message: hello\n")) {
+				t.Errorf("incorrect content: %s", cfg.Content)
+			}
+			if test.installationsErr != nil && creator.calls != 0 {
+				t.Errorf("client creator was called after installation lookup failed")
+			}
+			if test.creatorErr != nil && creator.calls != 1 {
+				t.Errorf("expected one client creation attempt, got %d", creator.calls)
+			}
+		})
+	}
+}
+
+func TestPrivateRemotesSameOwnerReusesOriginalClient(t *testing.T) {
+	client := makeTestClient()
+	installations := &testInstallationsService{}
+	creator := &testClientCreator{}
+	loader := NewLoader(nil, WithPrivateRemotes(creator, installations))
+
+	got := loader.remoteClient(context.Background(), client, "Source-Owner", "source-owner", "config")
+	if got != client {
+		t.Error("same-owner remote did not reuse the original client")
+	}
+	if installations.calls != 0 || creator.calls != 0 {
+		t.Errorf("same-owner remote attempted installation lookup or client creation: %d, %d", installations.calls, creator.calls)
+	}
+}
+
 func makeTestClient() *github.Client {
 	rp := &ResponsePlayer{}
 	for route, f := range map[string]string{

appconfig/options.go

@@ -14,6 +14,10 @@
 
 package appconfig
 
+import (
+	"github.com/palantir/go-githubapp/githubapp"
+)
+
 type Option func(*Loader)
 
 // WithRemoteRefParser sets the parser for encoded RemoteRefs. The default
@@ -36,20 +40,25 @@ func WithOwnerDefault(name string, paths []string) Option {
 	}
 }
 
-/*
-
-Not sure this is valuable yet, but leaving this option function as a starting
-point for a future implementation. See https://github.com/palantir/policy-bot/issues/111
-for some explanation of why this is desired.
-
-In the Loader implementation, if a ClientCreator and InstallationsService are
-set, the loadRemoteConfig method would use them to create a new client if the
-remote owner does not equal the starting owner.
-
 // WithPrivateRemotes enables loading remote configuration from private
-// repositories in different organizations. By default, only public
-// repositories can be remote targets.
-func WithPrivateRemotes(cc githubapp.ClientCreator, installs githubapp.InstallationsService) Option {
-	return func(ld *Loader) {}
+// repositories owned by a different user or organization. It uses the app's
+// installation on the remote repository to fetch the referenced file. If the
+// app is not installed on that repository, or an installation client cannot be created,
+// the loader logs the failure and falls back to the original client so public
+// remote repositories remain supported.
+//
+// WARNING: Enabling this option can expose configuration file content and
+// repository existence to users who otherwise may not be able to access them.
+// Enable it only when the app is installed on GitHub organizations where all
+// users are trusted, or when the caller otherwise prevents unintentional
+// information disclosure.
+//
+// This loader does not cache installation lookups or clients. Callers that
+// load configuration frequently should pass caching implementations, such as
+// githubapp.NewCachingInstallationsService and githubapp.NewCachingClientCreator.
+func WithPrivateRemotes(clientCreator githubapp.ClientCreator, installations githubapp.InstallationsService) Option {
+	return func(ld *Loader) {
+		ld.clientCreator = clientCreator
+		ld.installations = installations
+	}
 }
-*/