diff --git a/pkg/analyzer/analyzers/github/github.go b/pkg/analyzer/analyzers/github/github.go index e84d958fd76c..6c50e990fd0a 100644 --- a/pkg/analyzer/analyzers/github/github.go +++ b/pkg/analyzer/analyzers/github/github.go @@ -83,11 +83,11 @@ func secretInfoToUserBindings(info *common.SecretInfo) []analyzers.Binding { } func userToResource(user *gh.User) *analyzers.Resource { - name := *user.Login + name := user.GetLogin() return &analyzers.Resource{ Name: name, FullyQualifiedName: fmt.Sprintf("github.com/%s", name), - Type: strings.ToLower(*user.Type), // "user" or "organization" + Type: strings.ToLower(user.GetType()), // "user" or "organization" } } @@ -114,9 +114,14 @@ func secretInfoToRepoBindings(info *common.SecretInfo) []analyzers.Binding { } var bindings []analyzers.Binding for _, repo := range repos { + // A repo without an owner cannot be attributed; skip it rather than + // producing a corrupt parent resource with empty name/FQN. + if repo.GetOwner() == nil { + continue + } resource := analyzers.Resource{ - Name: *repo.Name, - FullyQualifiedName: fmt.Sprintf("github.com/%s", *repo.FullName), + Name: repo.GetName(), + FullyQualifiedName: fmt.Sprintf("github.com/%s", repo.GetFullName()), Type: "repository", Parent: userToResource(repo.Owner), } @@ -128,11 +133,17 @@ func secretInfoToRepoBindings(info *common.SecretInfo) []analyzers.Binding { func secretInfoToGistBindings(info *common.SecretInfo) []analyzers.Binding { var bindings []analyzers.Binding for _, gist := range info.Gists { + // A gist without an owner cannot be attributed to a user, so we cannot + // build a meaningful resource or FQN for it. Skip it rather than + // producing silently corrupt output (e.g. "gist.github.com//id"). + if gist.GetOwner() == nil { + continue + } resource := analyzers.Resource{ - Name: *gist.Description, - FullyQualifiedName: fmt.Sprintf("gist.github.com/%s/%s", *gist.Owner.Login, *gist.ID), + Name: gist.GetDescription(), + FullyQualifiedName: fmt.Sprintf("gist.github.com/%s/%s", gist.GetOwner().GetLogin(), gist.GetID()), Type: "gist", - Parent: userToResource(gist.Owner), + Parent: userToResource(gist.GetOwner()), } bindings = append(bindings, analyzers.BindAllPermissions(resource, info.Metadata.OauthScopes...)...) } diff --git a/pkg/analyzer/analyzers/github/github_test.go b/pkg/analyzer/analyzers/github/github_test.go index a3b9a9ee2e90..3d63ae848bd0 100644 --- a/pkg/analyzer/analyzers/github/github_test.go +++ b/pkg/analyzer/analyzers/github/github_test.go @@ -6,11 +6,95 @@ import ( "time" "github.com/google/go-cmp/cmp" + gh "github.com/google/go-github/v67/github" "github.com/trufflesecurity/trufflehog/v3/pkg/analyzer/analyzers" + githubcommon "github.com/trufflesecurity/trufflehog/v3/pkg/analyzer/analyzers/github/common" "github.com/trufflesecurity/trufflehog/v3/pkg/common" "github.com/trufflesecurity/trufflehog/v3/pkg/context" ) +func strPtr(s string) *string { return &s } + +func makeOwner(login, userType string) *gh.User { + return &gh.User{Login: &login, Type: &userType} +} + +// TestSecretInfoToGistBindings exercises nil-field edge cases that caused a +// production panic (nil Description and nil Owner from the GitHub API). +func TestSecretInfoToGistBindings(t *testing.T) { + owner := makeOwner("truffle-sandbox", "User") + scope := []analyzers.Permission{{Value: "gist"}} + + tests := []struct { + name string + gists []*gh.Gist + wantLen int + wantFQNs []string + wantNames []string // parallel to wantFQNs; checked unconditionally (including empty string) + }{ + { + name: "nil description produces empty name, gist is still included", + gists: []*gh.Gist{ + {ID: strPtr("abc123"), Description: nil, Owner: owner}, + }, + wantLen: 1, + wantFQNs: []string{"gist.github.com/truffle-sandbox/abc123"}, + wantNames: []string{""}, + }, + { + name: "nil owner: gist is skipped entirely", + gists: []*gh.Gist{ + {ID: strPtr("abc123"), Description: strPtr("my gist"), Owner: nil}, + }, + wantLen: 0, + }, + { + name: "mix of valid and nil-owner gists: only valid gist is included", + gists: []*gh.Gist{ + {ID: strPtr("abc123"), Description: strPtr("valid gist"), Owner: owner}, + {ID: strPtr("def456"), Description: strPtr("no owner"), Owner: nil}, + }, + wantLen: 1, + wantFQNs: []string{"gist.github.com/truffle-sandbox/abc123"}, + wantNames: []string{"valid gist"}, + }, + { + name: "empty gist list: no bindings", + gists: nil, + wantLen: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + info := &githubcommon.SecretInfo{ + Metadata: &githubcommon.TokenMetadata{OauthScopes: scope}, + Gists: tt.gists, + } + got := secretInfoToGistBindings(info) + if len(got) != tt.wantLen { + t.Errorf("got %d bindings, want %d", len(got), tt.wantLen) + } + for i, fqn := range tt.wantFQNs { + if i >= len(got) { + break + } + if got[i].Resource.FullyQualifiedName != fqn { + t.Errorf("binding[%d].FullyQualifiedName = %q, want %q", i, got[i].Resource.FullyQualifiedName, fqn) + } + } + for i, name := range tt.wantNames { + if i >= len(got) { + break + } + if got[i].Resource.Name != name { + t.Errorf("binding[%d].Name = %q, want %q", i, got[i].Resource.Name, name) + } + } + }) + } +} + func TestAnalyzer_Analyze(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) defer cancel()