Skip to content

feat(serde): support Git repo cloning via GitHub app#2246

Open
ivaaaan wants to merge 1 commit intoredpanda-data:masterfrom
ivaaaan:feat/support-github-app-authentication
Open

feat(serde): support Git repo cloning via GitHub app#2246
ivaaaan wants to merge 1 commit intoredpanda-data:masterfrom
ivaaaan:feat/support-github-app-authentication

Conversation

@ivaaaan
Copy link

@ivaaaan ivaaaan commented Feb 26, 2026

Currently to use protos from a private GitHub Repo you must setup either PAT or SSH key. Both methods are not recommended for organizations. This PR adds support for authentication via GitHub apps, which is a common way to authenticate apps in organizations.

Copilot AI review requested due to automatic review settings February 26, 2026 17:09
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds support for authenticating Git repository access using GitHub Apps, providing a more secure and organization-friendly authentication method compared to Personal Access Tokens (PATs) or SSH keys. The implementation follows GitHub's App authentication flow by generating short-lived installation access tokens using JWT-signed requests.

Changes:

  • Added GitHub App authentication configuration with validation for App ID, Installation ID, and private key
  • Implemented token generation and refresh logic with proper caching and concurrency controls
  • Integrated the new authentication method into the Git service alongside existing SSH and BasicAuth options

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
backend/pkg/config/github_auth_app.go New configuration struct for GitHub App authentication with validation
backend/pkg/config/git.go Integration of GitHub App config into main Git configuration
backend/pkg/config/git_auth_ssh.go Unintended whitespace changes
backend/pkg/git/github_app_auth.go Core implementation of GitHub App authentication with JWT signing and token management
backend/pkg/git/github_app_auth_test.go Comprehensive test suite covering key parsing, token refresh, and authentication
backend/pkg/git/service.go Integration of GitHub App auth into the service's authentication switch
backend/go.mod Updated golang-jwt dependency from indirect to direct
Comments suppressed due to low confidence (6)

backend/pkg/config/github_auth_app.go:46

  • The error messages use lowercase "github" while the brand name is "GitHub" (capital H) as seen in other comments and log messages throughout the codebase. Consider updating the error messages to use "GitHub" for consistency with the brand name.
		return errors.New("github app authentication is enabled but appId is not set or invalid")
	}

	if c.InstallationID <= 0 {
		return errors.New("github app authentication is enabled but installationId is not set or invalid")
	}

	if c.PrivateKey == "" && c.PrivateKeyFilePath == "" {
		return errors.New("github app authentication is enabled but neither privateKey nor privateKeyFilepath is set")

backend/pkg/config/git_auth_ssh.go:29

  • This file has unnecessary blank lines added at the end. These lines appear to be accidental and should be removed to keep the file clean and consistent with the rest of the codebase.
    backend/pkg/config/git.go:68
  • The Git configuration allows multiple authentication methods to be enabled simultaneously (SSH, BasicAuth, and GithubApp), but the switch statement in service.go will silently use only the first enabled method. Consider adding validation to the Git.Validate method to ensure only one authentication method is enabled at a time, similar to how SASL mechanisms are validated. This would prevent configuration errors and make the behavior more explicit to users.
	if err := c.GithubApp.Validate(); err != nil {
		return err
	}

backend/pkg/git/github_app_auth_test.go:180

  • The TestGitHubAppAuth_SetAuth test only covers the success case. Consider adding a test case for when getValidToken fails to ensure the error is properly logged and the request continues without authentication being set. This would verify the error handling behavior matches expectations.
func TestGitHubAppAuth_SetAuth(t *testing.T) {
	key := generateTestRSAKey(t)

	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.WriteHeader(http.StatusCreated)
		json.NewEncoder(w).Encode(installationTokenResponse{
			Token:     "ghs_test_token",
			ExpiresAt: time.Now().Add(1 * time.Hour),
		})
	}))
	defer server.Close()

	auth := &gitHubAppAuth{
		appID:          1,
		installationID: 2,
		privateKey:     key,
		logger:         slog.Default(),
		apiBaseURL:     server.URL,
	}

	req := httptest.NewRequest(http.MethodGet, "https://github.com/test/repo.git", nil)
	auth.SetAuth(req)

	username, password, ok := req.BasicAuth()
	require.True(t, ok)
	assert.Equal(t, "x-access-token", username)
	assert.Equal(t, "ghs_test_token", password)
}

backend/pkg/git/github_app_auth.go:106

  • The refreshToken method uses http.DefaultClient which has no timeout configured. This could cause the application to hang indefinitely if the GitHub API is unresponsive. Consider creating a custom HTTP client with appropriate timeout settings, similar to how backend/pkg/bsr/client.go creates a client with a 30-second timeout. For example, create a client with timeout in the buildGithubAppAuth function and store it in the gitHubAppAuth struct.
	resp, err := http.DefaultClient.Do(req)

backend/pkg/git/github_app_auth.go:187

  • Consider adding tests for the buildGithubAppAuth function to cover error cases such as invalid private key data and file reading failures. This would ensure the configuration loading logic is properly tested.
func buildGithubAppAuth(cfg config.GitGithubApp, logger *slog.Logger) (*gitHubAppAuth, error) {
	pemData, err := loadPrivateKeyPEM(cfg)
	if err != nil {
		return nil, err
	}

	privateKey, err := parseRSAPrivateKey(pemData)
	if err != nil {
		return nil, fmt.Errorf("failed to parse GitHub App private key: %w", err)
	}

	return &gitHubAppAuth{
		appID:          cfg.AppID,
		installationID: cfg.InstallationID,
		privateKey:     privateKey,
		logger:         logger,
		apiBaseURL:     gitHubAPIBaseURL,
	}, nil
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants