-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[INS-280] Fix Github "repostories" filter does not respect GHES endpoint #4677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…n err in ensureRepoInCache
pkg/sources/github/github.go
Outdated
| if u, err := url.Parse(repo); err == nil { | ||
| parts := strings.Split(u.Path, "/") | ||
| if len(parts) >= 2 { | ||
| repoName = parts[len(parts)-2] + "/" + strings.TrimSuffix(parts[len(parts)-1], ".git") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential trailing slash bug: if we ever get input like https://github.com/owner/repo.git/ for example, then this will give us repo.git/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
➕ I would probably use strings.TrimRight to remove any trailing slashes and then path.Base to just get the base name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I didn't touch it because I didn't wanna change existing logic, but I guess yeah, this can be made better
camgunz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the small trailing slash thing--maybe add a test case for that too
pkg/sources/github/github.go
Outdated
| if u, err := url.Parse(repo); err == nil { | ||
| parts := strings.Split(u.Path, "/") | ||
| if len(parts) >= 2 { | ||
| repoName = parts[len(parts)-2] + "/" + strings.TrimSuffix(parts[len(parts)-1], ".git") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
➕ I would probably use strings.TrimRight to remove any trailing slashes and then path.Base to just get the base name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| // we want to remove any path components from the endpoint and just use the host | ||
| u.Path = "/" + repo | ||
| fullURL = u.String() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enterprise check uses wrong condition for standard GitHub
High Severity
The normalizeRepo function uses s.conn.Endpoint != "" to detect GitHub Enterprise, but this condition is also true when users explicitly set the endpoint to "https://api.github.com" (standard GitHub API). In this case, the function incorrectly generates repository URLs using api.github.com as the host (e.g., https://api.github.com/org/repo) instead of the correct github.com (e.g., https://github.com/org/repo). The connector code correctly handles this by checking if the endpoint equals cloudV3Endpoint or matches endsWithGithub, but normalizeRepo does not use the same logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this related to this?
Description:
When a TruffleHog user attempts to filter on specific repositories within Github using the repositories configuration, the scanner does not respect the endpoint configured and instead defaults to using github.com.
After some investigation, I found that this was because of the fact that the
normalizeRepofunction doesn't respect theEndpointfield and always prefixes the repos withhttps://github.com. This causes thefilteredRepoCacheto be incorrectly set. Even though the client is configured to use the endpoint, mismatch with the cache causes the scan to fail.Checklist:
make test-community)?make lintthis requires golangci-lint)?