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>
This commit is contained in:
Rongsong Shen 2026-10-10 08:32:04 +00:00
parent 42dd1e7a5e
commit 42a402a5b4
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
6 changed files with 105 additions and 2 deletions

View file

@ -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 {