Fix GitHub enumeration & rate-limiting logic

[rgmz]

Description:

This is a follow-up to #2379.

It fixes the following issues:

1. GitHub API calls missing rate-limit handling
2. The fix for Refactor GitHub source #2379 (comment) inadvertently resulting in duplicate API calls

Checklist:

• Tests passing (make test-community)?
• Lint passing (make lint this requires golangci-lint)?

[rosecodym]

This change itself seems fine, but this feels squirrely enough that I feel obliged as a review to ask if there's any low-hanging test fruit we can harvest here. (The answer might be "no," but I want to check.)

[rgmz]

This change itself seems fine, but this feels squirrely enough that I feel obliged as a review to ask if there's any low-hanging test fruit we can harvest here. (The answer might be "no," but I want to check.)

I had a bit more time so I tweaked one of the tests to account for this. It could probably be better written.

[rosecodym]

@joeleonjr has identified a regression cased by #2379 that is affected by this pr, so i'm holding it up until he can write up a proper bug report so that we can figure out what's going on.

[rosecodym]

According to @joeleonjr, this PR changes one of the two errors he reported in #2640. With this PR applied:

the second one has the same error. The first one has a new error! Unable to cache repository info {"source_manager_worker_id": "ATqG1", "repo": "https://gist.github.com/274463.git", "error": "missing cached info for gist: https://gist.github.com/274463.git"}

@rgmz could you take a look at whether we can fix the regression here? If not, I think it would be good to take a step back and figure out the best path forward. (Which might be merging this PR as-is and opening a new one to fix the regression.)

removing my old after the bug report was created

[rgmz]

@joeleonjr try with the latest commit

[joeleonjr]

This type of Gist URL works now: https://gist.github.com/274463.git, but the longer one with a username still errors: failed to scan repository {"error": "no repoInfo for URL: https://gist.github.com/raccoons-bot/627e15a45a596068ce8bfef3fd05ccdb.git"}

[rgmz]

It seems like GitHub normalizes the clone URL to https://gist.github.com/<id>.git, regardless of whether you do https://gist.github.com/627e15a45a596068ce8bfef3fd05ccdb.git or https://gist.github.com/raccoons-bot/627e15a45a596068ce8bfef3fd05ccdb.git.

The logic caches the Gist with the URL returned by GitHub (https://gist.github.com/627e15a45a596068ce8bfef3fd05ccdb.git), but attempts to fetch it with the user-provided URL (https://gist.github.com/raccoons-bot/627e15a45a596068ce8bfef3fd05ccdb.git).

trufflehog/pkg/sources/github/repo.go

Lines 300 to 310 in 62ca9bb

	func (s *Source) cacheGistInfo(g *github.Gist) {
	info := repoInfo{
	owner: g.GetOwner().GetLogin(),
	}
	if g.GetPublic() {
	info.visibility = source_metadatapb.Visibility_public
	} else {
	info.visibility = source_metadatapb.Visibility_private
	}
	s.repoInfoCache.put(g.GetGitPullURL(), info)
	}

[rosecodym]

@rgmz If I'm correct that this is a strict improvement over what's currently out there - and that you've updated the tests to include the gist case that you fixed - I'm inclined to get this in and work on the second regression as a separate work item. Any objections?

[rgmz]

No, that makes sense to me.

[joeleonjr]

One thing to note: although running trufflehog github --repo https://gist.github.com/274463.git works, if you exclude the .git (which I imagine lots of users might do, since it's not in the URL bar that they'd be copying from), it throws a different error:

2024-03-29T09:24:18-04:00	error	trufflehog	invalid repository	{"repo": "https://gist.github.com/274463", "error": "Github repo appears to be missing the repo name. Org: \"274463\" Repo url: \"https://gist.github.com/274463\"", "errorVerbose": "Github repo appears to be missing the repo name. Org: \"274463\" Repo url: \"https://gist.github.com/274463\"

[rosecodym]

One thing to note: although running trufflehog github --repo https://gist.github.com/274463.git works, if you exclude the .git (which I imagine lots of users might do, since it's not in the URL bar that they'd be copying from), it throws a different error:

2024-03-29T09:24:18-04:00	error	trufflehog	invalid repository	{"repo": "https://gist.github.com/274463", "error": "Github repo appears to be missing the repo name. Org: \"274463\" Repo url: \"https://gist.github.com/274463\"", "errorVerbose": "Github repo appears to be missing the repo name. Org: \"274463\" Repo url: \"https://gist.github.com/274463\"

Thanks for catching this! Would you mind adding it to your original bug report, or creating a new one and linking them?

[rgmz]

One thing to note: although running trufflehog github --repo https://gist.github.com/274463.git works, if you exclude the .git (which I imagine lots of users might do, since it's not in the URL bar that they'd be copying from), it throws a different error:

2024-03-29T09:24:18-04:00	error	trufflehog	invalid repository	{"repo": "https://gist.github.com/274463", "error": "Github repo appears to be missing the repo name. Org: \"274463\" Repo url: \"https://gist.github.com/274463\"", "errorVerbose": "Github repo appears to be missing the repo name. Org: \"274463\" Repo url: \"https://gist.github.com/274463\"

@joeleonjr this appears to be an issue with giturl.go and, as far as I can tell, existed prior to #2379.

If the url includes .git it assumes that it's well-formatted and returns early.

trufflehog/pkg/giturl/giturl.go

Lines 70 to 72 in 0d3023f

	if strings.HasSuffix(repoURL, ".git") {
	return repoURL, nil
	}

Without the .git suffix, it fails validation.

trufflehog/pkg/giturl/giturl.go

Lines 94 to 96 in 0d3023f

	} else {
	return "", errors.Errorf("%s repo appears to be missing the repo name. Org: %q Repo url: %q", provider, org, repoURL)
	}