fix: prevent github webhook fetching wrong author info by ogzhanolguncu · Pull Request #5307 · unkeyed/unkey
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@svc/ctrl/api/github_webhook.go`:
- Around line 196-205: The avatar URL construction can produce a malformed URL
when both authorCommit.Author.Username and .Name are empty; update the logic
around authorHandle (used when calling githubclient.CommitInfoFromRaw and when
building the avatar URL string "https://github.com/%s.png") to detect an empty
authorHandle and pass a nil/empty avatar URL instead of formatting
"https://github.com/.png". Locate the block that sets authorHandle and the call
to githubclient.CommitInfoFromRaw (referenced as headCommit.ID,
headCommit.Message, authorHandle) and conditionally set the avatar URL to
empty/null when authorHandle == "" before invoking CommitInfoFromRaw.
---
Nitpick comments:
In `@svc/ctrl/api/github_webhook_integration_test.go`:
- Around line 32-57: Add a direct-push unit test mirroring
TestGitHubWebhook_Push_TriggersHandlePush but using a new payload where
Commits[0] == HeadCommit to exercise the direct-push path: create
TestGitHubWebhook_Push_DirectPush that sets up pushRequests channel and harness
via newWebhookHarness (same Services using mockGitHubWebhookService), call
sendWebhook with mustMarshal(newDirectPushPayload(testRepoFullName)) and
harness.Secret, then read from pushRequests and assert expected fields (e.g.,
req.GetCommitAuthorHandle() == "direct-pusher", repo/installation IDs,
branch/ref, after, commit message, author avatar, and commit timestamp) to
verify HandlePush is invoked for direct pushes; implement helper
newDirectPushPayload(repoFullName) returning a pushPayload with a single commit
and HeadCommit pointing to it.