diff --git a/internal/analyze/forge/auth_check_test.go b/internal/analyze/forge/auth_check_test.go new file mode 100644 index 0000000000..95f58eb6a9 --- /dev/null +++ b/internal/analyze/forge/auth_check_test.go @@ -0,0 +1,185 @@ +package forge + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" + + "github.com/google/go-github/v66/github" + "github.com/stretchr/testify/require" +) + +// fakeGitHub serves the three endpoints Check calls, as a GitHub Enterprise +// host would (under /api/v3/), each answering with the status it is given. +type fakeGitHub struct { + userStatus, repoStatus, rateStatus int + calls []string +} + +func (f *fakeGitHub) ServeHTTP(w http.ResponseWriter, r *http.Request) { + f.calls = append(f.calls, r.URL.Path) + w.Header().Set("Content-Type", "application/json") + reply := func(status int, body string) { + w.WriteHeader(status) + if status == http.StatusOK { + _, _ = fmt.Fprint(w, body) + return + } + _, _ = fmt.Fprintf(w, `{"message":"%s"}`, http.StatusText(status)) + } + switch r.URL.Path { + case "/api/v3/user": + reply(f.userStatus, `{"login":"octocat"}`) + case "/api/v3/repos/acme/widgets": + reply(f.repoStatus, `{"name":"widgets","owner":{"login":"acme"}}`) + case "/api/v3/rate_limit": + reply(f.rateStatus, `{"resources":{"core":{"limit":5000,"remaining":4321,"reset":1767225600}}}`) + default: + http.NotFound(w, r) + } +} + +// startFakeGitHub serves f over TLS and returns the host to pass to Check or NewClient. The +// client Check builds uses http.DefaultTransport, which is pointed at the test +// server's trusted transport for the duration of the test. +func startFakeGitHub(t *testing.T, f http.Handler) string { + t.Helper() + srv := httptest.NewTLSServer(f) + t.Cleanup(srv.Close) + prev := http.DefaultTransport + http.DefaultTransport = srv.Client().Transport + t.Cleanup(func() { http.DefaultTransport = prev }) + u, err := url.Parse(srv.URL) + require.NoError(t, err) + return u.Host +} + +var testCreds = &Credentials{Token: "t0ken", Source: "GH_TOKEN"} +var testRepo = Repo{Owner: "acme", Name: "widgets"} + +func TestCheck_UserTokenWithRepoAccess(t *testing.T) { + host := startFakeGitHub(t, &fakeGitHub{userStatus: 200, repoStatus: 200, rateStatus: 200}) + + res, err := Check(context.Background(), testCreds, host, testRepo) + require.NoError(t, err) + require.Equal(t, "octocat", res.Login) + require.Equal(t, "GH_TOKEN", res.Source) + require.Equal(t, 4321, res.RateLimitRemaining) + require.Equal(t, 5000, res.RateLimitLimit) + require.Equal(t, time.Unix(1767225600, 0).UTC(), res.RateLimitResetsAt.UTC()) +} + +func TestCheck_UserRejectedWithoutRepoIsATokenError(t *testing.T) { + f := &fakeGitHub{userStatus: 401, repoStatus: 200, rateStatus: 200} + host := startFakeGitHub(t, f) + + _, err := Check(context.Background(), testCreds, host, Repo{}) + require.Error(t, err) + require.Contains(t, err.Error(), "GitHub rejected the token from GH_TOKEN") + require.NotContains(t, f.calls, "/api/v3/repos/acme/widgets", "with no repository there is nothing else to try") +} + +// GitHub Actions' GITHUB_TOKEN cannot read /user; reading the repository is +// the validity check, and the result has no login. +func TestCheck_RepositoryScopedToken(t *testing.T) { + host := startFakeGitHub(t, &fakeGitHub{userStatus: 403, repoStatus: 200, rateStatus: 200}) + + res, err := Check(context.Background(), testCreds, host, testRepo) + require.NoError(t, err) + require.Empty(t, res.Login) + require.Equal(t, 4321, res.RateLimitRemaining) + require.Contains(t, res.String(), "repository-scoped token via GH_TOKEN") +} + +func TestCheck_RepositoryScopedTokenWithoutRateLimit(t *testing.T) { + host := startFakeGitHub(t, &fakeGitHub{userStatus: 403, repoStatus: 200, rateStatus: 500}) + + res, err := Check(context.Background(), testCreds, host, testRepo) + require.NoError(t, err) + require.Equal(t, CheckResult{Source: "GH_TOKEN"}, *res) +} + +func TestCheck_RepositoryScopedTokenThatCannotSeeTheRepo(t *testing.T) { + host := startFakeGitHub(t, &fakeGitHub{userStatus: 403, repoStatus: 404, rateStatus: 200}) + + _, err := Check(context.Background(), testCreds, host, testRepo) + require.Error(t, err) + require.Contains(t, err.Error(), "authenticated as repository-scoped token") +} + +// A valid user token that cannot see a private repository gets 404, not 403; +// the error must say it is an access problem and name the token and login. +func TestCheck_UserTokenCannotSeeTheRepo(t *testing.T) { + host := startFakeGitHub(t, &fakeGitHub{userStatus: 200, repoStatus: 404, rateStatus: 200}) + + _, err := Check(context.Background(), testCreds, host, testRepo) + require.Error(t, err) + msg := err.Error() + require.Contains(t, msg, "the GitHub token from GH_TOKEN is valid (authenticated as octocat) but cannot read acme/widgets") + require.Contains(t, msg, "almost certainly\n an access problem") +} + +// The quota is informational: failing to read it does not fail the check. +func TestCheck_RateLimitFailureIsNotFatal(t *testing.T) { + host := startFakeGitHub(t, &fakeGitHub{userStatus: 200, repoStatus: 200, rateStatus: 500}) + + res, err := Check(context.Background(), testCreds, host, testRepo) + require.NoError(t, err) + require.Equal(t, CheckResult{Login: "octocat", Source: "GH_TOKEN"}, *res) +} + +func TestRepoAccessError(t *testing.T) { + raw := errors.New("boom") + resp := func(code int) *github.Response { + return &github.Response{Response: &http.Response{StatusCode: code}} + } + + for _, code := range []int{http.StatusNotFound, http.StatusForbidden} { + err := repoAccessError(testCreds, "octocat", testRepo, resp(code), raw) + require.Contains(t, err.Error(), "the GitHub token from GH_TOKEN is valid (authenticated as octocat) but cannot read acme/widgets", "status %d", code) + require.NotErrorIs(t, err, raw, "the access message replaces the bare status") + } + + for _, r := range []*github.Response{resp(http.StatusInternalServerError), nil} { + err := repoAccessError(testCreds, "octocat", testRepo, r, raw) + require.Equal(t, "GitHub could not read acme/widgets: boom", err.Error()) + require.ErrorIs(t, err, raw, "any other failure keeps the raw error") + } +} + +func TestComma(t *testing.T) { + for n, want := range map[int]string{ + 0: "0", 7: "7", 999: "999", 1000: "1,000", 12345: "12,345", + 123456: "123,456", 1234567: "1,234,567", + } { + require.Equal(t, want, comma(n), "comma(%d)", n) + } +} + +func TestCheckResultString(t *testing.T) { + user := CheckResult{Login: "octocat", Source: "gh CLI", RateLimitRemaining: 4321, RateLimitLimit: 5000} + require.Equal(t, "GitHub authentication: ok (octocat via gh CLI, 4,321/5,000 requests remaining)", user.String()) + + scoped := CheckResult{Source: "GITHUB_TOKEN", RateLimitRemaining: 999, RateLimitLimit: 1000} + require.Equal(t, "GitHub authentication: ok (repository-scoped token via GITHUB_TOKEN, 999/1,000 requests remaining)", scoped.String()) +} + +func TestNewClientEnterpriseHost(t *testing.T) { + c, err := NewClient(testCreds, "ghe.example.com") + require.NoError(t, err) + require.Equal(t, "https://ghe.example.com/api/v3/", c.GitHub().BaseURL.String()) + + c, err = NewClient(testCreds, "github.com") + require.NoError(t, err) + require.Equal(t, "https://api.github.com/", c.GitHub().BaseURL.String(), "the public host keeps the default API URL") + + _, err = NewClient(testCreds, "bad host%zz") + require.Error(t, err) + require.True(t, strings.HasPrefix(err.Error(), "configure GitHub Enterprise host bad host%zz:"), err.Error()) +} diff --git a/internal/analyze/forge/fetch_test.go b/internal/analyze/forge/fetch_test.go new file mode 100644 index 0000000000..d69b7c0191 --- /dev/null +++ b/internal/analyze/forge/fetch_test.go @@ -0,0 +1,190 @@ +package forge + +import ( + "context" + "fmt" + "net/http" + "os" + "path/filepath" + "runtime" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// ghMux is a GitHub Enterprise API (under /api/v3/) answering fixed JSON per +// path. A path with no route is a 404. +func ghMux(routes map[string]string) http.Handler { + mux := http.NewServeMux() + for path, body := range routes { + body := body + mux.HandleFunc("/api/v3"+path, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprint(w, body) + }) + } + return mux +} + +func fakeClient(t *testing.T, routes map[string]string) *Client { + t.Helper() + host := startFakeGitHub(t, ghMux(routes)) + c, err := NewClient(testCreds, host) + require.NoError(t, err) + return c +} + +var ( + t0 = time.Date(2026, 10, 1, 9, 0, 0, 0, time.UTC) + at = func(d time.Duration) string { return t0.Add(d).Format(time.RFC3339) } + fetchRp = Repo{Owner: "acme", Name: "widgets", DefaultBranch: "main"} +) + +func TestFetchPullRequestsHydratesStateReviewsAndResponses(t *testing.T) { + c := fakeClient(t, map[string]string{ + "/repos/acme/widgets/pulls": `[ + {"number":3,"title":"merged","html_url":"u3","user":{"login":"ann"},"created_at":"` + at(0) + `","updated_at":"` + at(72*time.Hour) + `","merged_at":"` + at(5*time.Hour) + `","closed_at":"` + at(5*time.Hour) + `","merged_by":{"login":"bob"},"additions":10,"deletions":2,"changed_files":1,"comments":2}, + {"number":2,"title":"rejected","user":{"login":"ann"},"created_at":"` + at(0) + `","updated_at":"` + at(48*time.Hour) + `","closed_at":"` + at(6*time.Hour) + `"}, + {"number":1,"title":"too old","user":{"login":"ann"},"created_at":"` + at(-200*time.Hour) + `","updated_at":"` + at(-100*time.Hour) + `"} + ]`, + "/repos/acme/widgets/issues/3/timeline": `[]`, + "/repos/acme/widgets/issues/2/timeline": `[]`, + "/repos/acme/widgets/pulls/3/reviews": `[ + {"user":{"login":"ann"},"state":"COMMENTED","submitted_at":"` + at(30*time.Minute) + `"}, + {"user":{"login":"ci","type":"Bot"},"state":"COMMENTED","submitted_at":"` + at(40*time.Minute) + `"}, + {"user":{"login":"bob"},"state":"APPROVED","submitted_at":"` + at(3*time.Hour) + `"} + ]`, + "/repos/acme/widgets/pulls/2/reviews": `[]`, + "/repos/acme/widgets/issues/3/comments": `[ + {"user":{"login":"ann"},"created_at":"` + at(10*time.Minute) + `"}, + {"user":{"login":"dependabot[bot]","type":"Bot"},"created_at":"` + at(20*time.Minute) + `"}, + {"user":{"login":"cid"},"created_at":"` + at(2*time.Hour) + `"} + ]`, + "/repos/acme/widgets/issues/2/comments": `[]`, + }) + + prs, err := c.FetchPullRequests(context.Background(), fetchRp, t0) + require.NoError(t, err) + require.Len(t, prs, 2, "the PR updated before the window ends the walk") + + merged := prs[0] + require.Equal(t, 3, merged.Number) + require.Equal(t, "merged", merged.State) + require.Equal(t, "bob", merged.MergedBy.Login) + require.Equal(t, 10, merged.Additions) + require.Len(t, merged.Reviews, 3) + require.Equal(t, "approved", merged.Reviews[2].State, "review states are lower-cased") + require.Equal(t, t0.Add(3*time.Hour), merged.FirstReviewAt.UTC(), "the author's own and a bot's review do not count") + require.Equal(t, t0.Add(2*time.Hour), merged.FirstResponseAt.UTC(), "the first non-author, non-bot comment came before the review") + require.Equal(t, t0, merged.ReadyForReviewAt.UTC(), "never a draft: ready when opened") + + rejected := prs[1] + require.Equal(t, "closed", rejected.State, "closed without merging is a rejection, not a merge") + require.True(t, rejected.MergedAt.IsZero()) + require.True(t, rejected.FirstResponseAt.IsZero()) +} + +func TestDraftHistorySumsEveryDraftInterval(t *testing.T) { + c := fakeClient(t, map[string]string{ + "/repos/acme/widgets/pulls": `[{"number":7,"draft":false,"user":{"login":"ann"},"created_at":"` + at(0) + `","updated_at":"` + at(10*time.Hour) + `"}]`, + "/repos/acme/widgets/issues/7/timeline": `[ + {"event":"convert_to_draft","created_at":"` + at(1*time.Hour) + `"}, + {"event":"ready_for_review","created_at":"` + at(2*time.Hour) + `"}, + {"event":"convert_to_draft","created_at":"` + at(3*time.Hour) + `"}, + {"event":"ready_for_review","created_at":"` + at(5*time.Hour) + `"} + ]`, + "/repos/acme/widgets/pulls/7/reviews": `[]`, + "/repos/acme/widgets/issues/7/comments": `[ + {"user":{"login":"bob"},"created_at":"` + at(90*time.Minute) + `"} + ]`, + }) + + prs, err := c.FetchPullRequests(context.Background(), fetchRp, t0) + require.NoError(t, err) + require.Len(t, prs, 1) + p := prs[0] + require.Equal(t, t0.Add(2*time.Hour), p.ReadyForReviewAt.UTC(), "ready at the first ready_for_review") + require.Equal(t, int((3 * time.Hour).Seconds()), p.TimeInDraftSeconds, "1h + 2h across both draft cycles") + require.True(t, p.FirstResponseAt.IsZero(), "a comment before the PR was ready is not a response to it") +} + +func TestFetchIssuesSkipsPullRequests(t *testing.T) { + c := fakeClient(t, map[string]string{ + "/repos/acme/widgets/issues": `[ + {"number":10,"title":"bug","state":"closed","state_reason":"completed","user":{"login":"ann"},"labels":[{"name":"bug"},{"name":"p1"}],"created_at":"` + at(0) + `","closed_at":"` + at(time.Hour) + `"}, + {"number":11,"title":"a PR","user":{"login":"ann"},"pull_request":{"url":"x"},"created_at":"` + at(0) + `"} + ]`, + "/repos/acme/widgets/issues/10/comments": `[{"user":{"login":"bob"},"created_at":"` + at(30*time.Minute) + `"}]`, + }) + + issues, err := c.FetchIssues(context.Background(), fetchRp, t0) + require.NoError(t, err) + require.Len(t, issues, 1, "the PR the issues endpoint returns is not an issue") + is := issues[0] + require.Equal(t, 10, is.Number) + require.Equal(t, "completed", is.StateReason) + require.Equal(t, []string{"bug", "p1"}, is.Labels) + require.Equal(t, t0.Add(30*time.Minute), is.FirstResponseAt.UTC()) +} + +func TestFetchCommitReviews(t *testing.T) { + c := fakeClient(t, map[string]string{ + // approved on a PR into main + "/repos/acme/widgets/commits/aaa/pulls": `[ + {"number":20,"html_url":"u20","user":{"login":"ann"},"base":{"ref":"release"}}, + {"number":21,"html_url":"u21","user":{"login":"ann"},"base":{"ref":"main"}} + ]`, + "/repos/acme/widgets/pulls/21/reviews": `[{"user":{"login":"bob"},"state":"APPROVED","submitted_at":"` + at(0) + `"}]`, + // a PR into main nobody but the author approved + "/repos/acme/widgets/commits/bbb/pulls": `[{"number":22,"user":{"login":"ann"},"base":{"ref":"main"}}]`, + "/repos/acme/widgets/pulls/22/reviews": `[{"user":{"login":"ann"},"state":"APPROVED","submitted_at":"` + at(0) + `"}]`, + // no PR into main at all + "/repos/acme/widgets/commits/ccc/pulls": `[{"number":23,"user":{"login":"ann"},"base":{"ref":"release"}}]`, + // "ddd" has no route: the lookup fails and the commit is unknown + }) + + got, err := c.FetchCommitReviews(context.Background(), fetchRp, []string{"aaa", "bbb", "ccc", "ddd"}) + require.NoError(t, err) + require.Equal(t, []CommitReview{ + {SHA: "aaa", PRNumber: 21, PRURL: "u21", Status: "approved"}, + {SHA: "bbb", PRNumber: 22, Status: "review_required"}, + {SHA: "ccc", Status: "unknown"}, + {SHA: "ddd", Status: "unknown"}, + }, got) +} + +// fakeGh puts a gh on PATH that runs script. +func fakeGh(t *testing.T, script string) { + t.Helper() + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "gh"), []byte("#!/bin/sh\n"+script+"\n"), 0o755)) + t.Setenv("PATH", dir) +} + +func TestResolveCredentials_GhCLI(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("uses a shell script as gh") + } + t.Setenv("GITHUB_TOKEN", "") + t.Setenv("GH_TOKEN", "") + + fakeGh(t, `[ "$1 $2" = "auth token" ] && echo " from-gh-cli "`) + creds, err := ResolveCredentials(context.Background()) + require.NoError(t, err) + require.Equal(t, "from-gh-cli", creds.Token, "gh's output is trimmed") + require.Equal(t, "gh CLI", creds.Source) + + for name, script := range map[string]string{ + "gh fails": "exit 1", + "gh prints blank": "echo", + } { + fakeGh(t, script) + _, err := ResolveCredentials(context.Background()) + require.ErrorIs(t, err, ErrNoCredentials, name) + } + + t.Setenv("PATH", t.TempDir()) // no gh at all + _, err = ResolveCredentials(context.Background()) + require.ErrorIs(t, err, ErrNoCredentials, "gh absent") +}