fix(github): resolve panic & test failures (#2608)

trufflesecurity · Mar 22, 2024 · 9d4cf87 · 9d4cf87
1 parent 6dbe808
commit 9d4cf87
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 11 deletions.
diff --git a/pkg/sources/errors.go b/pkg/sources/errors.go
@@ -20,6 +20,10 @@ func NewScanErrors() *ScanErrors {
 
 // Add an error to the collection in a thread-safe manner.
 func (s *ScanErrors) Add(err error) {
+	if err == nil {
+		return
+	}
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	s.errors = append(s.errors, err)

diff --git a/pkg/sources/errors_test.go b/pkg/sources/errors_test.go
@@ -152,7 +152,7 @@ func TestScanErrorsCount(t *testing.T) {
 func TestScanErrorsString(t *testing.T) {
 	se := NewScanErrors()
 	se.Add(nil)
-	want := "[<nil>]"
+	want := "[]"
 	if got := fmt.Sprintf("%v", se); got != want {
 		t.Errorf("got %q, want %q", got, want)
 	}

diff --git a/pkg/sources/github/github.go b/pkg/sources/github/github.go
@@ -417,12 +417,19 @@ func (s *Source) enumerate(ctx context.Context, apiEndpoint string) (*github.Cli
 			continue
 		}
 
-		ghRepo, _, err := s.apiClient.Repositories.Get(ctx, urlParts[1], urlParts[2])
-		if err != nil {
-			ctx.Logger().Error(err, "failed to fetch repository")
-			continue
+		// Ignore any gists in |s.filteredRepoCache|.
+		// Repos have three parts (github.com, owner, name), gists have two.
+		if len(urlParts) == 3 {
+			// Ensure that individual repos specified in --repo are cached.
+			// Gists should be cached elsewhere.
+			// https://github.com/trufflesecurity/trufflehog/pull/2379#discussion_r1487454788
+			ghRepo, _, err := s.apiClient.Repositories.Get(ctx, urlParts[1], urlParts[2])
+			if err != nil {
+				ctx.Logger().Error(err, "failed to fetch repository")
+				continue
+			}
+			s.cacheRepoInfo(ghRepo)
 		}
-		s.cacheRepoInfo(ghRepo)
 		s.repos = append(s.repos, r)
 	}
 	githubReposEnumerated.WithLabelValues(s.name).Set(float64(len(s.repos)))

diff --git a/pkg/sources/github/github_test.go b/pkg/sources/github/github_test.go
@@ -433,18 +433,23 @@ func TestEnumerate(t *testing.T) {
 	gock.New("https://api.github.com").
 		Get("/users/super-secret-user/repos").
 		Reply(200).
-		JSON([]map[string]string{{"clone_url": "https://github.com/super-secret-repo.git", "full_name": "super-secret-repo"}})
+		JSON([]map[string]string{{"clone_url": "https://github.com/super-secret-user/super-secret-repo.git", "full_name": "super-secret-user/super-secret-repo"}})
+
+	gock.New("https://api.github.com").
+		Get("/repos/super-secret-user/super-secret-repo").
+		Reply(200).
+		JSON(`{"owner": {"login": "super-secret-user"}, "name": "super-secret-repo", "full_name": "super-secret-user/super-secret-repo", "has_wiki": false, "size": 1}`)
 
 	gock.New("https://api.github.com").
 		Get("/user/orgs").
 		MatchParam("per_page", "100").
 		Reply(200).
-		JSON([]map[string]string{{"clone_url": "https://github.com/super-secret-repo.git", "full_name": "super-secret-repo"}})
+		JSON([]map[string]string{{"clone_url": "https://github.com/super-secret-user/super-secret-repo.git", "full_name": "super-secret-user/super-secret-repo"}})
 
 	gock.New("https://api.github.com").
 		Get("/users/super-secret-user/gists").
 		Reply(200).
-		JSON([]map[string]string{{"git_pull_url": "https://github.com/super-secret-gist.git", "id": "super-secret-gist"}})
+		JSON(`[{"git_pull_url": "https://gist.github.com/2801a2b0523099d0614a951579d99ba9.git", "id": "2801a2b0523099d0614a951579d99ba9"}]`)
 
 	s := initTestSource(&sourcespb.GitHub{
 		Credential: &sourcespb.GitHub_Token{
@@ -455,9 +460,9 @@ func TestEnumerate(t *testing.T) {
 	_, err := s.enumerate(context.Background(), "https://api.github.com")
 	assert.Nil(t, err)
 	assert.Equal(t, 2, s.filteredRepoCache.Count())
-	ok := s.filteredRepoCache.Exists("super-secret-repo")
+	ok := s.filteredRepoCache.Exists("super-secret-user/super-secret-repo")
 	assert.True(t, ok)
-	ok = s.filteredRepoCache.Exists("super-secret-gist")
+	ok = s.filteredRepoCache.Exists("2801a2b0523099d0614a951579d99ba9")
 	assert.True(t, ok)
 	assert.True(t, gock.IsDone())
 }