refactor improve NoBetterThan (#26126)

- The `NoBetterThan` function can only handle comparisons between
"pending," "success," "error," and "failure." For any other comparison,
we directly return false. This prevents logic errors like the one in
#26121.
- The callers of the `NoBetterThan` function should also avoid making
incomparable calls.

---------

Co-authored-by: yp05327 <576951401@qq.com>
Co-authored-by: puni9869 <80308335+puni9869@users.noreply.github.com>

1 files changed, 17 insertions, 6 deletions

diff --git a/modules/structs/commit_status.go b/modules/structs/commit_status.go
index ff31f2d2ac..de1d8fa566 100644
--- a/modules/structs/commit_status.go
+++ b/modules/structs/commit_status.go
@@ -4,7 +4,7 @@
 package structs
 
 // CommitStatusState holds the state of a CommitStatus
-// It can be "pending", "success", "error", "failure", and "warning"
+// It can be "pending", "success", "error" and "failure"
 type CommitStatusState string
 
 const (
@@ -18,14 +18,25 @@ const (
 	CommitStatusFailure CommitStatusState = "failure"
 )
 
+var commitStatusPriorities = map[CommitStatusState]int{
+	CommitStatusError:   0,
+	CommitStatusFailure: 1,
+	CommitStatusPending: 2,
+	CommitStatusSuccess: 3,
+}
+
 // NoBetterThan returns true if this State is no better than the given State
+// This function only handles the states defined in CommitStatusPriorities
 func (css CommitStatusState) NoBetterThan(css2 CommitStatusState) bool {
-	commitStatusPriorities := map[CommitStatusState]int{
-		CommitStatusError:   0,
-		CommitStatusFailure: 1,
-		CommitStatusPending: 2,
-		CommitStatusSuccess: 3,
+	// NoBetterThan only handles the 4 states above
+	if _, exist := commitStatusPriorities[css]; !exist {
+		return false
 	}
+
+	if _, exist := commitStatusPriorities[css2]; !exist {
+		return false
+	}
+
 	return commitStatusPriorities[css] <= commitStatusPriorities[css2]
 }