From fd179b891ba5ffd199adb93444f77bc8318be9e0 Mon Sep 17 00:00:00 2001 From: Rodin Date: Sat, 2 May 2026 07:11:57 -0700 Subject: [PATCH] fix: detect shared-token misconfiguration and warn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When two review-bot roles share the same Gitea user token (misconfiguration), log a WARNING identifying which sibling is sharing. The bot continues normally with its own honest verdict — no escalation, no deadlock. Operators see the warning in CI logs and can fix the token setup. Addresses Aaron's review feedback on #28: graceful degradation when someone doesn't follow the separate-token deployment instructions. --- cmd/review-bot/main.go | 42 ++++++++++++++++++++++++ cmd/review-bot/main_test.go | 65 +++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index aed9ba4..daeef3f 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -266,6 +266,9 @@ func main() { if err != nil { log.Printf("Warning: could not list existing reviews: %v", err) } else { + // Detect shared-token misconfiguration: warn if sibling sentinel exists from same user + warnSharedToken(existingReviews, sentinel) + existing := findOwnReview(existingReviews, sentinel) if existing != nil { @@ -487,6 +490,45 @@ func reviewUnchanged(reviews []gitea.Review, newBody, newEvent, sentinel string) return false } +// warnSharedToken logs a warning if another review-bot role posted under the +// same Gitea user. This detects misconfiguration where two roles share a token +// instead of having separate Gitea accounts. The bot continues normally (posts +// its own honest verdict) but warns operators to fix the token setup. +func warnSharedToken(reviews []gitea.Review, ownSentinel string) { + ownLogin := "" + for _, r := range reviews { + if strings.Contains(r.Body, ownSentinel) { + ownLogin = r.User.Login + break + } + } + if ownLogin == "" { + return + } + for _, r := range reviews { + if r.User.Login == ownLogin && strings.Contains(r.Body, "" + idx := strings.Index(body, prefix) + if idx < 0 { + return "unknown" + } + rest := body[idx+len(prefix):] + end := strings.Index(rest, suffix) + if end < 0 { + return "unknown" + } + return rest[:end] +} + // findOwnReview locates a review matching the given sentinel in its body. func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review { for i := range reviews { diff --git a/cmd/review-bot/main_test.go b/cmd/review-bot/main_test.go index 6633f10..2643eb7 100644 --- a/cmd/review-bot/main_test.go +++ b/cmd/review-bot/main_test.go @@ -188,3 +188,68 @@ func TestFindOwnReview(t *testing.T) { }) } } + +func TestWarnSharedToken(t *testing.T) { + // warnSharedToken should not panic and should handle edge cases gracefully. + // It only logs — we verify it doesn't crash. + tests := []struct { + name string + reviews []gitea.Review + sentinel string + }{ + { + name: "no reviews", + reviews: nil, + sentinel: "", + }, + { + name: "no own review yet", + reviews: []gitea.Review{ + {ID: 1, User: struct{ Login string `json:"login"` }{Login: "other"}, Body: " body"}, + }, + sentinel: "", + }, + { + name: "separate users - no warning", + reviews: []gitea.Review{ + {ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: " body"}, + {ID: 2, User: struct{ Login string `json:"login"` }{Login: "security-review-bot"}, Body: " body"}, + }, + sentinel: "", + }, + { + name: "shared token detected", + reviews: []gitea.Review{ + {ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: " body"}, + {ID: 2, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: " body"}, + }, + sentinel: "", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Should not panic + warnSharedToken(tc.reviews, tc.sentinel) + }) + } +} + +func TestExtractSentinelName(t *testing.T) { + tests := []struct { + body string + want string + }{ + {" rest", "sonnet"}, + {" rest", "security"}, + {"no sentinel here", "unknown"}, + {" rest", "gpt-review"}, + } + + for _, tc := range tests { + got := extractSentinelName(tc.body) + if got != tc.want { + t.Errorf("extractSentinelName(%q) = %q, want %q", tc.body, got, tc.want) + } + } +}