Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions pkg/analyzer/analyzers/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}

Expand All @@ -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 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Skipping a repo resource just because we don't know about it's owner doesn't seem right to me. Maybe we should set the parent resource as nil for such cases.

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),
}
Expand All @@ -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...)...)
}
Expand Down
84 changes: 84 additions & 0 deletions pkg/analyzer/analyzers/github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading