diff --git a/db_github.go b/db_github.go index 3ab2b79..eb2d57a 100644 --- a/db_github.go +++ b/db_github.go @@ -1,6 +1,8 @@ // Package dalgo2ghingitdb provides a DALgo database adapter for reading inGitDB repositories from GitHub using the GitHub API. // It supports read-only access to public repositories with no authentication required. -// Future versions will support authentication and write operations for private repositories. +// Authenticated access is configured either with a static Config.Token or, for +// rotating credentials such as short-lived GitHub App installation tokens, +// with a Config.TokenProvider that is consulted on every request. package dalgo2ghingitdb import ( diff --git a/file_reader.go b/file_reader.go index 7d2bca5..2b69591 100644 --- a/file_reader.go +++ b/file_reader.go @@ -12,10 +12,20 @@ import ( // Config defines connection settings for reading an inGitDB repository from GitHub. type Config struct { - Owner string - Repo string - Ref string - Token string + Owner string + Repo string + Ref string + + // Token is a static GitHub token (e.g. a classic/fine-grained PAT). + // If TokenProvider is nil and Token is non-empty, Token is wrapped in + // StaticTokenProvider automatically, preserving legacy behavior. + Token string + + // TokenProvider, when set, supplies the token per request and takes + // precedence over Token. Use it to inject rotating credentials such as + // short-lived GitHub App installation tokens. + TokenProvider TokenProvider + APIBaseURL string HTTPClient *http.Client } @@ -30,6 +40,19 @@ func (c Config) validate() error { return nil } +// tokenProvider resolves the effective TokenProvider: an explicit +// Config.TokenProvider wins; otherwise a non-empty Config.Token is wrapped in +// a StaticTokenProvider; nil means unauthenticated requests. +func (c Config) tokenProvider() TokenProvider { + if c.TokenProvider != nil { + return c.TokenProvider + } + if c.Token != "" { + return StaticTokenProvider(c.Token) + } + return nil +} + // FileReader reads repository files by path from GitHub. type FileReader interface { ReadFile(ctx context.Context, path string) (content []byte, found bool, err error) @@ -46,25 +69,9 @@ func NewGitHubFileReader(cfg Config) (FileReader, error) { if err != nil { return nil, err } - opts := make([]github.ClientOptionsFunc, 0, 3) // max 3: HTTPClient + Token + APIBaseURL - httpClient := cfg.HTTPClient - if httpClient == nil { - httpClient = http.DefaultClient - } - opts = append(opts, github.WithHTTPClient(httpClient)) - if cfg.Token != "" { - opts = append(opts, github.WithAuthToken(cfg.Token)) - } - if cfg.APIBaseURL != "" { - baseURL := cfg.APIBaseURL - if !strings.HasSuffix(baseURL, "/") { - baseURL += "/" - } - opts = append(opts, github.WithEnterpriseURLs(baseURL, baseURL)) - } - client, err := github.NewClient(opts...) + client, err := newGitHubAPIClient(cfg) if err != nil { - return nil, fmt.Errorf("failed to create github client: %w", err) + return nil, err } return &githubFileReader{cfg: cfg, client: client}, nil } diff --git a/github_client.go b/github_client.go new file mode 100644 index 0000000..442e4b2 --- /dev/null +++ b/github_client.go @@ -0,0 +1,50 @@ +package dalgo2ghingitdb + +import ( + "fmt" + "net/http" + "strings" + + "github.com/google/go-github/v88/github" +) + +// newGitHubAPIClient builds the go-github client from a Config. Shared by +// NewGitHubFileReader and NewTreeWriter (which previously duplicated this +// construction logic inline). +// +// Fable refactoring: authorization moved from a construction-time +// github.WithAuthToken(cfg.Token) to the per-request tokenProviderTransport, +// so tokens can rotate per operation (short-lived GitHub App installation +// tokens; see TokenProvider). A bare Config.Token keeps working unchanged — +// Config.tokenProvider() wraps it in StaticTokenProvider. +func newGitHubAPIClient(cfg Config) (*github.Client, error) { + httpClient := cfg.HTTPClient + if httpClient == nil { + httpClient = http.DefaultClient + } + if provider := cfg.tokenProvider(); provider != nil { + base := httpClient.Transport + if base == nil { + base = http.DefaultTransport + } + // Shallow-copy the client so a caller-supplied http.Client is never + // mutated by the adapter. + authClient := *httpClient + authClient.Transport = tokenProviderTransport{base: base, provider: provider} + httpClient = &authClient + } + opts := make([]github.ClientOptionsFunc, 0, 2) // max 2: HTTPClient + APIBaseURL + opts = append(opts, github.WithHTTPClient(httpClient)) + if cfg.APIBaseURL != "" { + baseURL := cfg.APIBaseURL + if !strings.HasSuffix(baseURL, "/") { + baseURL += "/" + } + opts = append(opts, github.WithEnterpriseURLs(baseURL, baseURL)) + } + client, err := github.NewClient(opts...) + if err != nil { + return nil, fmt.Errorf("failed to create github client: %w", err) + } + return client, nil +} diff --git a/go.sum b/go.sum index d9dcffa..c791282 100644 --- a/go.sum +++ b/go.sum @@ -4,6 +4,7 @@ github.com/bits-and-blooms/bitset v1.24.4 h1:95H15Og1clikBrKr/DuzMXkQzECs1M6hhoG github.com/bits-and-blooms/bitset v1.24.4/go.mod h1:7hO7Gc7Pp1vODcmWvKMRA9BNmbv6a/7QIWpPxHddWR8= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/dal-go/dalgo v0.62.2 h1:rA+k9QCgS0UnZV0oonvDtA+QoM3Wj1doVBWo1higBTw= +github.com/dal-go/dalgo v0.62.2/go.mod h1:wwaibB/UlzwAY1r9KZw5JCis+aroIwAu5WQJ/aoWZiU= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= @@ -41,6 +42,7 @@ github.com/strongo/random v0.0.1/go.mod h1:/pSI+SjBNLBkjljNtVdYr6ERddA+LqSa87o0/ go.starlark.net v0.0.0-20260613233743-8ba36ccb83fb h1:NGUBN0jbH0IR3msRslALnoxlySm+6YvVKvVDjdDJrlA= go.starlark.net v0.0.0-20260613233743-8ba36ccb83fb/go.mod h1:Iue6g6iirlfLoVi/DYCi5/x0h/bAOuWF3dULTKpt2Vo= golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/token_provider.go b/token_provider.go new file mode 100644 index 0000000..5b27645 --- /dev/null +++ b/token_provider.go @@ -0,0 +1,67 @@ +package dalgo2ghingitdb + +import ( + "context" + "fmt" + "net/http" +) + +// TokenProvider supplies the GitHub token used to authorize API calls. +// It is invoked once per outgoing HTTP request, so implementations can rotate +// tokens (e.g. mint short-lived GitHub App installation tokens on demand) +// without rebuilding the adapter. Returning an empty token with a nil error +// sends the request unauthenticated (fine for public-repo reads). +// +// See docs/roadmaps/ovdb-access-tokens-grants.md in sneat-co/backstage +// (Decision 3.4): the sneat-go backend injects a provider that mints 1-hour +// installation tokens with an in-memory cache; CLI/dev injects +// StaticTokenProvider with a PAT. +type TokenProvider interface { + Token(ctx context.Context) (string, error) +} + +// TokenProviderFunc adapts a plain function to the TokenProvider interface. +type TokenProviderFunc func(ctx context.Context) (string, error) + +// Token implements TokenProvider. +func (f TokenProviderFunc) Token(ctx context.Context) (string, error) { + return f(ctx) +} + +// StaticTokenProvider returns a TokenProvider that always yields the given +// fixed token (e.g. a personal access token for CLI/dev usage). It preserves +// the legacy Config.Token behavior: a non-empty Config.Token is wrapped in a +// StaticTokenProvider automatically when Config.TokenProvider is nil. +func StaticTokenProvider(token string) TokenProvider { + return staticTokenProvider(token) +} + +type staticTokenProvider string + +func (p staticTokenProvider) Token(context.Context) (string, error) { + return string(p), nil +} + +// tokenProviderTransport is an http.RoundTripper that asks the TokenProvider +// for a token on every request and injects it as a Bearer Authorization +// header. Provider errors abort the request and surface to the caller as +// regular operation errors (wrapped by the http.Client, then by +// wrapGitHubError at the call sites). +type tokenProviderTransport struct { + base http.RoundTripper + provider TokenProvider +} + +func (t tokenProviderTransport) RoundTrip(req *http.Request) (*http.Response, error) { + token, err := t.provider.Token(req.Context()) + if err != nil { + return nil, fmt.Errorf("token provider failed: %w", err) + } + if token != "" { + // RoundTrippers must not mutate the caller's request; clone before + // setting the header. + req = req.Clone(req.Context()) + req.Header.Set("Authorization", "Bearer "+token) + } + return t.base.RoundTrip(req) +} diff --git a/token_provider_test.go b/token_provider_test.go new file mode 100644 index 0000000..329aea6 --- /dev/null +++ b/token_provider_test.go @@ -0,0 +1,339 @@ +package dalgo2ghingitdb + +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" +) + +func TestStaticTokenProvider(t *testing.T) { + t.Parallel() + provider := StaticTokenProvider("my-static-token") + for i := 0; i < 2; i++ { + token, err := provider.Token(context.Background()) + if err != nil { + t.Fatalf("Token() unexpected error: %v", err) + } + if token != "my-static-token" { + t.Errorf("Token() = %q, want %q", token, "my-static-token") + } + } +} + +func TestTokenProviderFunc(t *testing.T) { + t.Parallel() + wantErr := errors.New("boom") + provider := TokenProviderFunc(func(ctx context.Context) (string, error) { + return "fn-token", wantErr + }) + token, err := provider.Token(context.Background()) + if token != "fn-token" { + t.Errorf("Token() = %q, want %q", token, "fn-token") + } + if !errors.Is(err, wantErr) { + t.Errorf("Token() error = %v, want %v", err, wantErr) + } +} + +func TestConfig_TokenProvider_Resolution(t *testing.T) { + t.Parallel() + explicit := StaticTokenProvider("explicit") + tests := []struct { + name string + cfg Config + want string // "" means expect nil provider + }{ + {name: "no token no provider", cfg: Config{}, want: ""}, + {name: "static token auto-wrapped", cfg: Config{Token: "legacy-pat"}, want: "legacy-pat"}, + {name: "explicit provider", cfg: Config{TokenProvider: explicit}, want: "explicit"}, + {name: "provider wins over token", cfg: Config{Token: "legacy-pat", TokenProvider: explicit}, want: "explicit"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + provider := tc.cfg.tokenProvider() + if tc.want == "" { + if provider != nil { + t.Fatalf("tokenProvider() = %v, want nil", provider) + } + return + } + if provider == nil { + t.Fatal("tokenProvider() = nil, want non-nil") + } + token, err := provider.Token(context.Background()) + if err != nil { + t.Fatalf("Token() unexpected error: %v", err) + } + if token != tc.want { + t.Errorf("Token() = %q, want %q", token, tc.want) + } + }) + } +} + +// authRecordingServer records the Authorization header of every request and +// serves a minimal GitHub Contents API file response. +func authRecordingServer(t *testing.T) (*httptest.Server, func() []string) { + t.Helper() + var mu sync.Mutex + var authHeaders []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + authHeaders = append(authHeaders, r.Header.Get("Authorization")) + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + response := map[string]any{ + "type": "file", + "encoding": "base64", + "content": base64.StdEncoding.EncodeToString([]byte("hello")), + "sha": "abc123", + } + if err := json.NewEncoder(w).Encode(response); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } + })) + t.Cleanup(server.Close) + return server, func() []string { + mu.Lock() + defer mu.Unlock() + return append([]string(nil), authHeaders...) + } +} + +func TestFileReader_StaticConfigToken_SendsBearerHeader(t *testing.T) { + t.Parallel() + server, headers := authRecordingServer(t) + + cfg := Config{Owner: "test", Repo: "test", Token: "legacy-pat", APIBaseURL: server.URL + "/"} + reader, err := NewGitHubFileReader(cfg) + if err != nil { + t.Fatalf("NewGitHubFileReader: %v", err) + } + if _, _, err = reader.ReadFile(context.Background(), "test.txt"); err != nil { + t.Fatalf("ReadFile: %v", err) + } + got := headers() + if len(got) != 1 { + t.Fatalf("expected 1 request, got %d", len(got)) + } + if got[0] != "Bearer legacy-pat" { + t.Errorf("Authorization = %q, want %q", got[0], "Bearer legacy-pat") + } +} + +func TestFileReader_TokenProvider_CalledPerOperation(t *testing.T) { + t.Parallel() + server, headers := authRecordingServer(t) + + var mu sync.Mutex + calls := 0 + provider := TokenProviderFunc(func(ctx context.Context) (string, error) { + mu.Lock() + defer mu.Unlock() + calls++ + return fmt.Sprintf("tok-%d", calls), nil + }) + + cfg := Config{Owner: "test", Repo: "test", TokenProvider: provider, APIBaseURL: server.URL + "/"} + reader, err := NewGitHubFileReader(cfg) + if err != nil { + t.Fatalf("NewGitHubFileReader: %v", err) + } + + ctx := context.Background() + for i := 0; i < 2; i++ { + if _, _, readErr := reader.ReadFile(ctx, "test.txt"); readErr != nil { + t.Fatalf("ReadFile #%d: %v", i+1, readErr) + } + } + + mu.Lock() + gotCalls := calls + mu.Unlock() + if gotCalls != 2 { + t.Errorf("provider called %d times, want 2 (once per operation)", gotCalls) + } + got := headers() + want := []string{"Bearer tok-1", "Bearer tok-2"} + if len(got) != len(want) { + t.Fatalf("expected %d requests, got %d", len(want), len(got)) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("request #%d Authorization = %q, want %q (token must rotate per operation)", i+1, got[i], want[i]) + } + } +} + +func TestFileReader_TokenProvider_WinsOverStaticToken(t *testing.T) { + t.Parallel() + server, headers := authRecordingServer(t) + + provider := StaticTokenProvider("provider-token") + cfg := Config{Owner: "test", Repo: "test", Token: "static-token", TokenProvider: provider, APIBaseURL: server.URL + "/"} + reader, err := NewGitHubFileReader(cfg) + if err != nil { + t.Fatalf("NewGitHubFileReader: %v", err) + } + if _, _, err = reader.ReadFile(context.Background(), "test.txt"); err != nil { + t.Fatalf("ReadFile: %v", err) + } + got := headers() + if len(got) != 1 || got[0] != "Bearer provider-token" { + t.Errorf("Authorization headers = %v, want [%q]", got, "Bearer provider-token") + } +} + +func TestFileReader_TokenProvider_EmptyTokenMeansUnauthenticated(t *testing.T) { + t.Parallel() + server, headers := authRecordingServer(t) + + provider := TokenProviderFunc(func(ctx context.Context) (string, error) { + return "", nil + }) + cfg := Config{Owner: "test", Repo: "test", TokenProvider: provider, APIBaseURL: server.URL + "/"} + reader, err := NewGitHubFileReader(cfg) + if err != nil { + t.Fatalf("NewGitHubFileReader: %v", err) + } + if _, _, err = reader.ReadFile(context.Background(), "test.txt"); err != nil { + t.Fatalf("ReadFile: %v", err) + } + got := headers() + if len(got) != 1 || got[0] != "" { + t.Errorf("Authorization headers = %v, want one empty header", got) + } +} + +func TestFileReader_TokenProvider_ErrorPropagates(t *testing.T) { + t.Parallel() + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + http.NotFound(w, r) + })) + defer server.Close() + + providerErr := errors.New("installation token minting failed") + provider := TokenProviderFunc(func(ctx context.Context) (string, error) { + return "", providerErr + }) + cfg := Config{Owner: "test", Repo: "test", TokenProvider: provider, APIBaseURL: server.URL + "/"} + reader, err := NewGitHubFileReader(cfg) + if err != nil { + t.Fatalf("NewGitHubFileReader: %v", err) + } + + _, _, err = reader.ReadFile(context.Background(), "test.txt") + if err == nil { + t.Fatal("ReadFile() expected error from token provider, got nil") + } + if !strings.Contains(err.Error(), "token provider failed") { + t.Errorf("ReadFile() error = %q, want to contain 'token provider failed'", err.Error()) + } + if !strings.Contains(err.Error(), providerErr.Error()) { + t.Errorf("ReadFile() error = %q, want to contain %q", err.Error(), providerErr.Error()) + } + if requests != 0 { + t.Errorf("server received %d requests, want 0 (request must not fire without a token)", requests) + } + + // ListDirectory goes through the same transport and must fail the same way. + if _, err = reader.ListDirectory(context.Background(), "some/dir"); err == nil { + t.Fatal("ListDirectory() expected error from token provider, got nil") + } else if !strings.Contains(err.Error(), "token provider failed") { + t.Errorf("ListDirectory() error = %q, want to contain 'token provider failed'", err.Error()) + } +} + +func TestTreeWriter_TokenProvider_ErrorPropagates(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer server.Close() + + providerErr := errors.New("grant revoked") + provider := TokenProviderFunc(func(ctx context.Context) (string, error) { + return "", providerErr + }) + cfg := Config{Owner: "test", Repo: "test", Ref: "main", TokenProvider: provider, APIBaseURL: server.URL + "/"} + writer, err := NewTreeWriter(cfg) + if err != nil { + t.Fatalf("NewTreeWriter: %v", err) + } + + _, err = writer.CommitChanges(context.Background(), "msg", []TreeChange{{Path: "a.txt", Content: []byte("x")}}) + if err == nil { + t.Fatal("CommitChanges() expected error from token provider, got nil") + } + if !strings.Contains(err.Error(), "token provider failed") { + t.Errorf("CommitChanges() error = %q, want to contain 'token provider failed'", err.Error()) + } + if !strings.Contains(err.Error(), providerErr.Error()) { + t.Errorf("CommitChanges() error = %q, want to contain %q", err.Error(), providerErr.Error()) + } +} + +func TestTreeWriter_TokenProvider_SendsBearerHeader(t *testing.T) { + t.Parallel() + var mu sync.Mutex + var authHeaders []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + authHeaders = append(authHeaders, r.Header.Get("Authorization")) + mu.Unlock() + http.NotFound(w, r) // the operation may fail; we only assert auth + })) + defer server.Close() + + cfg := Config{Owner: "test", Repo: "test", Ref: "main", TokenProvider: StaticTokenProvider("writer-token"), APIBaseURL: server.URL + "/"} + writer, err := NewTreeWriter(cfg) + if err != nil { + t.Fatalf("NewTreeWriter: %v", err) + } + + _, _ = writer.ListFilesUnder(context.Background(), "dir") + + mu.Lock() + got := append([]string(nil), authHeaders...) + mu.Unlock() + if len(got) == 0 { + t.Fatal("expected at least one request to reach the server") + } + for i, h := range got { + if h != "Bearer writer-token" { + t.Errorf("request #%d Authorization = %q, want %q", i+1, h, "Bearer writer-token") + } + } +} + +func TestFileReader_CustomHTTPClientNotMutated(t *testing.T) { + t.Parallel() + server, headers := authRecordingServer(t) + + custom := &http.Client{} + cfg := Config{Owner: "test", Repo: "test", Token: "tok", HTTPClient: custom, APIBaseURL: server.URL + "/"} + reader, err := NewGitHubFileReader(cfg) + if err != nil { + t.Fatalf("NewGitHubFileReader: %v", err) + } + if custom.Transport != nil { + t.Error("caller-supplied http.Client.Transport was mutated; adapter must copy the client") + } + if _, _, err = reader.ReadFile(context.Background(), "test.txt"); err != nil { + t.Fatalf("ReadFile: %v", err) + } + got := headers() + if len(got) != 1 || got[0] != "Bearer tok" { + t.Errorf("Authorization headers = %v, want [%q]", got, "Bearer tok") + } +} diff --git a/tree_writer.go b/tree_writer.go index 05cf27d..60869e3 100644 --- a/tree_writer.go +++ b/tree_writer.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "net/http" "strings" "github.com/google/go-github/v88/github" @@ -35,25 +34,9 @@ func NewTreeWriter(cfg Config) (*TreeWriter, error) { if err := cfg.validate(); err != nil { return nil, err } - opts := make([]github.ClientOptionsFunc, 0, 3) // max 3: HTTPClient + Token + APIBaseURL - httpClient := cfg.HTTPClient - if httpClient == nil { - httpClient = http.DefaultClient - } - opts = append(opts, github.WithHTTPClient(httpClient)) - if cfg.Token != "" { - opts = append(opts, github.WithAuthToken(cfg.Token)) - } - if cfg.APIBaseURL != "" { - baseURL := cfg.APIBaseURL - if !strings.HasSuffix(baseURL, "/") { - baseURL += "/" - } - opts = append(opts, github.WithEnterpriseURLs(baseURL, baseURL)) - } - client, err := github.NewClient(opts...) + client, err := newGitHubAPIClient(cfg) if err != nil { - return nil, fmt.Errorf("failed to create github client: %w", err) + return nil, err } return &TreeWriter{cfg: cfg, client: client}, nil }