Changes to fix Enterprise UI filtering of Github Hosted Scanner Repositories to Include (#4430)

* Changes to fix Enterprise UI filtering of Github Hosted Scanner Repositories to Include * trying to pass linting isssues * trying to solve linting errors * Added upstream repo filtering, improvements to normalizeRepo, and unit tests for both. * Fixing tests impacted by PR 4426
2025-09-03 13:16:39 -07:00
parent 2114e77d56
commit a66d9e75c2
3 changed files with 112 additions and 10 deletions
@@ -205,6 +205,11 @@ func (c *filteredRepoCache) includeRepo(s string) bool {
 	return false
 }

+// wantRepo returns true if the repository should be included based on include/exclude patterns
+func (c *filteredRepoCache) wantRepo(s string) bool {
+	return !c.ignoreRepo(s) && c.includeRepo(s)
+}
+
 // Init returns an initialized GitHub source.
 func (s *Source) Init(aCtx context.Context, name string, jobID sources.JobID, sourceID sources.SourceID, verify bool, connection *anypb.Any, concurrency int) error {
 	err := git.CmdCheck()
@@ -433,14 +438,29 @@ func (s *Source) Enumerate(ctx context.Context, reporter sources.UnitReporter) e
 	// Double make sure that all enumerated repositories in the
 	// filteredRepoCache have an entry in the repoInfoCache.
 	for _, repo := range s.filteredRepoCache.Values() {
-		ctx := context.WithValue(ctx, "repo", repo)
+		// Extract the repository name from the URL for filtering
+		repoName := repo
+		if strings.Contains(repo, "/") {
+			// Try to extract org/repo name from URL
+			if strings.Contains(repo, "github.com") {
+				parts := strings.Split(repo, "/")
+				if len(parts) >= 2 {
+					repoName = parts[len(parts)-2] + "/" + strings.TrimSuffix(parts[len(parts)-1], ".git")
+				}
+			}
+		}

-		repo, err := s.ensureRepoInfoCache(ctx, repo, &unitErrorReporter{reporter})
-		if err != nil {
-			ctx.Logger().Error(err, "error caching repo info")
-			_ = dedupeReporter.UnitErr(ctx, fmt.Errorf("error caching repo info: %w", err))
+		// Final filter check - only include repositories that pass the filter
+		if s.filteredRepoCache.wantRepo(repoName) {
+			ctx = context.WithValue(ctx, "repo", repo)
+
+			repo, err := s.ensureRepoInfoCache(ctx, repo, &unitErrorReporter{reporter})
+			if err != nil {
+				ctx.Logger().Error(err, "error caching repo info")
+				_ = dedupeReporter.UnitErr(ctx, fmt.Errorf("error caching repo info: %w", err))
+			}
+			s.repos = append(s.repos, repo)
 		}
-		s.repos = append(s.repos, repo)
 	}
 	githubReposEnumerated.WithLabelValues(s.name).Set(float64(len(s.repos)))
 	ctx.Logger().Info("Completed enumeration", "num_repos", len(s.repos), "num_orgs", s.orgsCache.Count(), "num_members", len(s.memberCache))
@@ -423,6 +423,41 @@ func TestNormalizeRepos(t *testing.T) {
 	}
 }

+func TestNormalizeRepo(t *testing.T) {
+	// Test that normalizeRepo correctly identifies URLs with protocols
+	source := &Source{}
+
+	// Test case 1: HTTP URL
+	result, err := source.normalizeRepo("https://github.com/org/repo.git")
+	assert.NoError(t, err)
+	assert.Contains(t, result, "github.com/org/repo")
+
+	// Test case 2: HTTP URL without .git
+	result, err = source.normalizeRepo("http://github.com/org/repo")
+	assert.NoError(t, err)
+	assert.Contains(t, result, "github.com/org/repo")
+
+	// Test case 3: Git protocol URL
+	result, err = source.normalizeRepo("git://github.com/org/repo.git")
+	assert.NoError(t, err)
+	assert.Contains(t, result, "github.com/org/repo")
+
+	// Test case 4: SSH URL
+	result, err = source.normalizeRepo("ssh://[email protected]/org/repo.git")
+	assert.NoError(t, err)
+	assert.Contains(t, result, "github.com/org/repo")
+
+	// Test case 5: Org/repo format (should convert to full URL)
+	result, err = source.normalizeRepo("org/repo")
+	assert.NoError(t, err)
+	assert.Contains(t, result, "github.com/org/repo")
+
+	// Test case 6: Invalid format (no protocol, no slash)
+	_, err = source.normalizeRepo("invalid")
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "no repositories found")
+}
+
 func TestHandleRateLimit(t *testing.T) {
 	s := initTestSource(&sourcespb.GitHub{Credential: &sourcespb.GitHub_Unauthenticated{}})
 	ctx := context.Background()
@@ -493,7 +528,7 @@ func TestEnumerateWithToken(t *testing.T) {
 		JSON(map[string]string{"login": "super-secret-user"})

 	gock.New("https://api.github.com").
-		Get("/users/super-secret-user/repos").
+		Get("/user/repos").
 		MatchParam("per_page", "100").
 		Reply(200).
 		JSON([]map[string]string{{"clone_url": "https://github.com/super-secret-user/super-secret-repo.git", "full_name": "super-secret-user/super-secret-repo"}})
@@ -574,7 +609,7 @@ func TestEnumerate(t *testing.T) {

 	//
 	gock.New("https://api.github.com").
-		Get("/users/super-secret-user/repos").
+		Get("/user/repos").
 		Reply(200).
 		JSON(`[{"name": "super-secret-repo", "full_name": "super-secret-user/super-secret-repo", "owner": {"login": "super-secret-user"}, "clone_url": "https://github.com/super-secret-user/super-secret-repo.git", "has_wiki": false, "size": 1}]`)

@@ -978,6 +1013,39 @@ func Test_ScanMultipleTargets_MultipleErrors(t *testing.T) {
 	}
 }

+func TestRepositoryFiltering(t *testing.T) {
+	// Test that the filteredRepoCache correctly filters repositories
+	source := &Source{}
+
+	// Test case 1: No filters specified (should include everything)
+	cache1 := source.newFilteredRepoCache(context.Background(), simple.NewCache[string](), []string{}, []string{})
+	assert.True(t, cache1.wantRepo("org/repo1"))
+	assert.True(t, cache1.wantRepo("org/repo2"))
+	assert.True(t, cache1.wantRepo("org/repo3"))
+
+	// Test case 2: Include filter specified (should only include matching repos)
+	cache2 := source.newFilteredRepoCache(context.Background(), simple.NewCache[string](), []string{"org/repo1", "org/repo2"}, []string{})
+	assert.True(t, cache2.wantRepo("org/repo1"))
+	assert.True(t, cache2.wantRepo("org/repo2"))
+	assert.False(t, cache2.wantRepo("org/repo3"))
+
+	// Test case 3: Exclude filter specified (should exclude matching repos)
+	cache3 := source.newFilteredRepoCache(context.Background(), simple.NewCache[string](), []string{}, []string{"org/repo1"})
+	assert.False(t, cache3.wantRepo("org/repo1"))
+	assert.True(t, cache3.wantRepo("org/repo2"))
+	assert.True(t, cache3.wantRepo("org/repo3"))
+
+	// Test case 4: Both include and exclude filters (exclude takes precedence)
+	cache4 := source.newFilteredRepoCache(context.Background(), simple.NewCache[string](), []string{"org/repo1"}, []string{"org/repo1"})
+	assert.False(t, cache4.wantRepo("org/repo1"))
+
+	// Test case 5: Wildcard patterns
+	cache5 := source.newFilteredRepoCache(context.Background(), simple.NewCache[string](), []string{"org/*"}, []string{})
+	assert.True(t, cache5.wantRepo("org/repo1"))
+	assert.True(t, cache5.wantRepo("org/repo2"))
+	assert.False(t, cache5.wantRepo("other/repo1"))
+}
+
 func noopReporter() sources.UnitReporter {
 	return sources.VisitorReporter{
 		VisitUnit: func(context.Context, sources.SourceUnit) error {
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"regexp"
 	"strings"
 	"sync"

@@ -273,6 +274,12 @@ func (s *Source) processRepos(ctx context.Context, target string, reporter sourc
 			}

 			repoName, repoURL := r.GetFullName(), r.GetCloneURL()
+
+			// Check if we should process this repository based on the filter
+			if !s.filteredRepoCache.wantRepo(repoName) {
+				continue
+			}
+
 			s.totalRepoSize += r.GetSize()
 			s.filteredRepoCache.Set(repoName, repoURL)
 			s.cacheRepoInfo(r)
@@ -349,10 +356,17 @@ func (s *Source) wikiIsReachable(ctx context.Context, repoURL string) bool {

 // normalizeRepo normalizes a GitHub repository URL or name to its canonical form.
 func (s *Source) normalizeRepo(repo string) (string, error) {
-	// if the string contains a '/', assume it's a GitHub repository.
-	if strings.ContainsRune(repo, '/') {
+
+	// If it's a full URL (has protocol), normalize it
+	if regexp.MustCompile(`^[a-z]+://`).MatchString(repo) {
+
 		return giturl.NormalizeGithubRepo(repo)
 	}
+	// If it's a repository name (contains / but not http), convert to full URL first
+	if strings.Contains(repo, "/") && !regexp.MustCompile(`^[a-z]+://`).MatchString(repo) {
+		fullURL := "https://github.com/" + repo
+		return giturl.NormalizeGithubRepo(fullURL)
+	}

 	return "", fmt.Errorf("no repositories found for %s", repo)
 }