From 400cccaed454692c0f15eb91cbdf4a98786621da Mon Sep 17 00:00:00 2001 From: fullsend-code Date: Thu, 28 May 2026 07:49:13 +0000 Subject: [PATCH] fix(#1640): fall back to /app endpoint for bot user detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetAuthenticatedUser calls GET /user to determine the authenticated identity. GitHub App installation tokens (used in WIF/OIDC mode) cannot call /user, causing two degraded behaviors in the review agent: marker spoofing protection is weakened and stale review cleanup is skipped entirely. When GET /user fails, fall back to GET /app which returns the app metadata including the slug. Construct the bot login as "{slug}[bot]" — the conventional GitHub bot username format. Tests added for: successful fallback, both endpoints failing, and /app returning an empty slug. Note: pre-commit could not run in sandbox due to Go toolchain permission error (go1.26.0 download blocked). All Go tests and vet passed using GOMODCACHE override. Closes #1640 --- internal/forge/github/github.go | 36 +++++++++++++--- internal/forge/github/github_test.go | 62 ++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/internal/forge/github/github.go b/internal/forge/github/github.go index 4786639d7..146aa2b3c 100644 --- a/internal/forge/github/github.go +++ b/internal/forge/github/github.go @@ -900,19 +900,43 @@ func (c *LiveClient) GetOrgPlan(ctx context.Context, org string) (string, error) } // GetAuthenticatedUser returns the login of the authenticated user. +// +// For classic PATs and OAuth tokens the identity comes from GET /user. +// GitHub App installation tokens cannot call /user, so when that call +// fails the method falls back to GET /app and constructs the +// conventional bot login "{slug}[bot]". func (c *LiveClient) GetAuthenticatedUser(ctx context.Context) (string, error) { resp, err := c.get(ctx, "/user") - if err != nil { + if err == nil { + var user struct { + Login string `json:"login"` + } + if err := decodeJSON(resp, &user); err != nil { + return "", fmt.Errorf("decode user: %w", err) + } + return user.Login, nil + } + + // /user is not available for GitHub App installation tokens. + // Fall back to /app which returns the app's metadata including + // its slug, from which we derive the bot login. + appResp, appErr := c.get(ctx, "/app") + if appErr != nil { + // Neither endpoint worked — return the original /user error + // because that is the more common path. return "", fmt.Errorf("get authenticated user: %w", err) } - var user struct { - Login string `json:"login"` + var app struct { + Slug string `json:"slug"` + } + if appErr := decodeJSON(appResp, &app); appErr != nil { + return "", fmt.Errorf("decode app: %w", appErr) } - if err := decodeJSON(resp, &user); err != nil { - return "", fmt.Errorf("decode user: %w", err) + if app.Slug == "" { + return "", fmt.Errorf("get authenticated user: /app returned empty slug") } - return user.Login, nil + return app.Slug + "[bot]", nil } // GetTokenScopes returns the OAuth scopes granted to the current token diff --git a/internal/forge/github/github_test.go b/internal/forge/github/github_test.go index acf7229f5..5081399bc 100644 --- a/internal/forge/github/github_test.go +++ b/internal/forge/github/github_test.go @@ -310,6 +310,68 @@ func TestGetAuthenticatedUser(t *testing.T) { assert.Equal(t, "test-bot", user) } +func TestGetAuthenticatedUser_FallbackToApp(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/user": + // Simulate GitHub App installation token: /user returns 403. + w.WriteHeader(http.StatusForbidden) + json.NewEncoder(w).Encode(map[string]any{ + "message": "Resource not accessible by integration", + }) + case "/app": + json.NewEncoder(w).Encode(map[string]any{ + "slug": "fullsend-ai-review", + }) + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + })) + defer srv.Close() + + client := newTestClient(t, srv) + user, err := client.GetAuthenticatedUser(context.Background()) + require.NoError(t, err) + assert.Equal(t, "fullsend-ai-review[bot]", user) +} + +func TestGetAuthenticatedUser_BothFail(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + json.NewEncoder(w).Encode(map[string]any{ + "message": "forbidden", + }) + })) + defer srv.Close() + + client := newTestClient(t, srv) + _, err := client.GetAuthenticatedUser(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "get authenticated user") +} + +func TestGetAuthenticatedUser_AppEmptySlug(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/user": + w.WriteHeader(http.StatusForbidden) + json.NewEncoder(w).Encode(map[string]any{ + "message": "forbidden", + }) + case "/app": + json.NewEncoder(w).Encode(map[string]any{ + "slug": "", + }) + } + })) + defer srv.Close() + + client := newTestClient(t, srv) + _, err := client.GetAuthenticatedUser(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "empty slug") +} + func TestCreateRepoSecret(t *testing.T) { callNum := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {