fix(apiserver): abort the request on every middleware failure (bug-1949)

Cause: ReturnError only writes the response and never aborts the gin
chain; RequirePermission (token error) and WhitelistIPMiddleware (invalid
client IP) returned after ReturnError without ctx.Abort(), so the route
handler still ran after the rejection.

Fix: ctx.Abort() after ReturnError at both sites (and, for clarity, on the
ValidateHandler body-check path, which validateRequestBody already
aborted). RequirePermission now answers a bad token with 401 instead of
500. ReturnError stays non-aborting (documented): handlers such as
ControlHost call it from helpers and then test ctx.IsAborted(), so a
global abort would change handler flows, and it adds nothing in handlers.

Tests: TestRequirePermission_AbortsOnInvalidToken,
TestWhitelistIPMiddleware_AbortsOnFailure.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/frontend/apiserver/controllers/rbac_abort_test.go

@@ -0,0 +1,48 @@
+package controllers
+
+import (
+	"apiserver/utils"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+	"go.uber.org/zap"
+)
+
+// bug-1949: a RequirePermission failure must stop the request before the
+// handler runs.
+func TestRequirePermission_AbortsOnInvalidToken(t *testing.T) {
+	resetRBACState()
+	prev := utils.AuthenticationYamlConfig
+	utils.AuthenticationYamlConfig = &utils.Authentication{
+		Token: utils.TokenConfig{Type: "jwt", SecretKey: "rbac-abort-test-secret", TokenExpireTime: 3600},
+	}
+	utils.Log = zap.NewNop()
+	t.Cleanup(func() { utils.AuthenticationYamlConfig = prev })
+
+	gin.SetMode(gin.TestMode)
+	r := gin.New()
+	handlerRan := false
+	r.GET("/protected", RequirePermission("host:view"), func(ctx *gin.Context) {
+		handlerRan = true
+		ReturnSuccess(ctx, http.StatusOK, "success", nil)
+	})
+
+	for _, header := range []string{"", "Bearer not-a-jwt"} {
+		handlerRan = false
+		req := httptest.NewRequest(http.MethodGet, "/protected", nil)
+		if header != "" {
+			req.Header.Set("Authorization", header)
+		}
+		w := httptest.NewRecorder()
+		r.ServeHTTP(w, req)
+
+		if handlerRan {
+			t.Errorf("Authorization %q: handler ran after RequirePermission failed (status %d)", header, w.Code)
+		}
+		if w.Code != http.StatusUnauthorized {
+			t.Errorf("Authorization %q: status = %d, want %d", header, w.Code, http.StatusUnauthorized)
+		}
+	}
+}

src/frontend/apiserver/controllers/rbac_check.go

@@ -117,7 +117,8 @@ func RequirePermission(perm string) gin.HandlerFunc {
 		token := ctx.GetHeader("Authorization")
 		logonUser, err := GetUsernameFromToken(token)
 		if err != nil {
-			ReturnError(ctx, common.ERROR_CODE, err.Error(), nil)
+			ReturnError(ctx, common.ERROR_AUTH_CHECK_TOKEN_CODE, err.Error(), nil)
+			ctx.Abort()
 			return
 		}
 		logger.Debug("Logon user is ", zap.String("logonUser", logonUser))

src/frontend/apiserver/controllers/response.go

@@ -27,6 +27,9 @@ func ReturnSuccess(ctx *gin.Context, code int, msg interface{}, data interface{}
 	ctx.JSON(200, json)
 }
 
+// ReturnError writes the error envelope. It does not abort the gin chain:
+// a middleware that rejects a request must call ctx.Abort() after it
+// (bug-1949); handlers just return.
 func ReturnError(ctx *gin.Context, code int, msg interface{}, details interface{}) {
 	json := &JsonErrStruct{
 		Code:    code,

src/frontend/apiserver/middleware/validatehandler.go

@@ -35,6 +35,7 @@ func ValidateHandler() gin.HandlerFunc {
 			if err != nil || !isValidBody {
 				controllers.ReturnError(ctx, common.INVALID_PARAMS_CODE,
 					common.ERROR_READ_REQUEST_BODY_MSG+": "+err.Error(), nil)
+				ctx.Abort()
 				return
 			}
 		}

src/frontend/apiserver/middleware/whitelist.go

@@ -129,6 +129,7 @@ func WhitelistIPMiddleware(allowedIPs []string) gin.HandlerFunc {
 		if clientIP == nil {
 			logger.Error("invalid client IP", zap.String("clientIP", clientIPStr))
 			controllers.ReturnError(ctx, common.INVALID_PARAMS_CODE, common.ERROR_INVALID_CLIENT_IP_MSG, nil)
+			ctx.Abort()
 			return
 		}
 

src/frontend/apiserver/middleware/whitelist_test.go

@@ -1,6 +1,55 @@
 package middleware
 
-import "testing"
+import (
+	"apiserver/utils"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+	"go.uber.org/zap"
+)
+
+// bug-1949: every IP-whitelist failure must stop the request before the
+// handler runs.
+func TestWhitelistIPMiddleware_AbortsOnFailure(t *testing.T) {
+	prevLog := utils.Log
+	utils.Log = zap.NewNop()
+	t.Cleanup(func() { utils.Log = prevLog })
+
+	tests := []struct {
+		name       string
+		remoteAddr string
+		wantStatus int
+	}{
+		{"unparsable client IP", "not-an-address", http.StatusBadRequest},
+		{"client IP not whitelisted", "192.0.2.10:4000", http.StatusForbidden},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gin.SetMode(gin.TestMode)
+			r := gin.New()
+			handlerRan := false
+			r.Use(WhitelistIPMiddleware([]string{"10.0.0.0/8"}))
+			r.GET("/ping", func(ctx *gin.Context) {
+				handlerRan = true
+				ctx.String(http.StatusOK, "pong")
+			})
+
+			req := httptest.NewRequest(http.MethodGet, "/ping", nil)
+			req.RemoteAddr = tt.remoteAddr
+			w := httptest.NewRecorder()
+			r.ServeHTTP(w, req)
+
+			if handlerRan {
+				t.Errorf("handler ran after the IP whitelist rejected %q (status %d)", tt.remoteAddr, w.Code)
+			}
+			if w.Code != tt.wantStatus {
+				t.Errorf("status = %d, want %d", w.Code, tt.wantStatus)
+			}
+		})
+	}
+}
 
 func TestPathMatchesPattern(t *testing.T) {
 	cases := []struct {