fix(apiserver): stop ControlHost after validateHost rejects the request (bug-1961)

Cause: validateHost (controllers/hosts.go) wrote a 403 via ReturnError
without aborting, so ControlHost's ctx.IsAborted() check never fired and it
went on to write a second (202) response; the permission was also checked
once per host and not at all for an empty host list.

Fix: validateHost writes no response; it checks host:control once up front
and returns an error, on which ControlHost writes a single 403 and returns.
For "all", the host list is resolved before the one validateHost call.

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

src/frontend/apiserver/controllers/hosts.go

@@ -422,17 +422,17 @@ func (h *HostHandler) ControlHost(ctx *gin.Context) {
 	successHosts := []map[string]interface{}{}
 	failedHosts := []map[string]interface{}{}
 
-	validatedSuccessHosts, validatedFailedHosts := validateHost(ctx, actionType, req.Hostname)
+	hostnames := req.Hostname
-	if ctx.IsAborted() {
+	if allFlag {
+		hostnames = store.GetAllHostNames()
+	}
+	validatedSuccessHosts, validatedFailedHosts, err := validateHost(ctx, actionType, hostnames)
+	if err != nil {
+		// validateHost rejected the request: answer once and stop (bug-1961).
 		ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil)
 		return
 	}
 
-	if allFlag {
-		hostnames := store.GetAllHostNames()
-		validatedSuccessHosts, validatedFailedHosts = validateHost(ctx, actionType, hostnames)
-	}
-
 	for _, hostname := range validatedSuccessHosts {
 		successHosts = append(successHosts, map[string]interface{}{
 			"host_name": hostname,
@@ -471,16 +471,27 @@ func (h *HostHandler) ControlHost(ctx *gin.Context) {
 	return
 }
 
-// validateHost validates the hostname in the request
+// validateHost validates the hostnames in the request. It writes no
-func validateHost(ctx *gin.Context, control int, hostnames []string) (successHosts []string, failedHosts map[string]string) {
+// response: a non-nil error means the caller lacks host:control (or its
+// token is invalid) and the caller must reject the whole request (bug-1961).
+func validateHost(ctx *gin.Context, control int, hostnames []string) (successHosts []string, failedHosts map[string]string, err error) {
 	logger.Debug("Validating hostnames: %v", zap.Any("hostnames", hostnames))
 
 	// get logon user
 	token := ctx.GetHeader("Authorization")
 	logonUser, err := GetUsernameFromToken(token)
 	if err != nil {
-		ReturnError(ctx, common.ERROR_CODE, err.Error(), nil)
+		logger.Error(err.Error())
-		return
+		return nil, nil, err
+	}
+
+	hostPermission, err := GetRequireResourcePermission(logonUser, "host:control")
+	if err != nil {
+		logger.Error(err.Error(), zap.String("user", logonUser))
+		return nil, nil, err
+	}
+	if hostPermission.DomainType == models.DomainTypeHostGroup {
+		// TODO: check hostgroup values
 	}
 
 	successHosts = []string{}
@@ -497,16 +508,6 @@ func validateHost(ctx *gin.Context, control int, hostnames []string) (successHos
 			continue
 		}
 
-		hostPermission, err := GetRequireResourcePermission(logonUser, "host:control")
-		if err != nil {
-			logger.Error(err.Error())
-			ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil)
-			return
-		}
-		if hostPermission.DomainType == models.DomainTypeHostGroup {
-			// TODO: check hostgroup values
-		}
-
 		if store.Hosts.Has(hostname) {
 			logger.Debug("Found hostname in store", zap.String("hostname", hostname))
 			successHosts = append(successHosts, hostname)

src/frontend/apiserver/controllers/hosts_handler_test.go

@@ -0,0 +1,68 @@
+//go:build !cgo
+
+package controllers
+
+import (
+	"encoding/json"
+	"io"
+	"net/http"
+	"strings"
+	"testing"
+)
+
+// RBAC fixture for bug-1961: viewer has no host:control grant at all.
+const hostControlRBAC = `
+permissions:
+  - host:control
+  - host:view
+roles:
+  host_viewer:
+    permissions:
+      - action: host:view
+        domain:
+          - type: all
+users:
+  viewer:
+    roles:
+      - name: host_viewer
+`
+
+// countJSONValues returns how many JSON documents body holds back to back:
+// a handler that writes a second response after an error shows up as 2.
+func countJSONValues(t *testing.T, body string) int {
+	t.Helper()
+	dec := json.NewDecoder(strings.NewReader(body))
+	n := 0
+	for {
+		var v interface{}
+		if err := dec.Decode(&v); err == io.EOF {
+			return n
+		} else if err != nil {
+			t.Fatalf("decoding response body %q: %v", body, err)
+		}
+		n++
+	}
+}
+
+// bug-1961: when validateHost rejects the request (no host:control grant),
+// ControlHost must stop with a single 403 response. The route is mounted
+// without RequirePermission so the handler's own check is what is tested.
+func TestControlHost_RejectedWritesSingleResponse(t *testing.T) {
+	for _, hosts := range []string{`["node01"]`, `["all"]`} {
+		t.Run(hosts, func(t *testing.T) {
+			setupRBACTest(t, hostControlRBAC)
+			r := newTestRouter()
+			h := &HostHandler{}
+			r.POST("/xce/v1/hosts/control", h.ControlHost)
+
+			w := doRoleRequest(r, http.MethodPost, "/xce/v1/hosts/control", "viewer",
+				`{"action":"close","host_name":`+hosts+`}`)
+			if n := countJSONValues(t, w.Body.String()); n != 1 {
+				t.Errorf("handler wrote %d responses, want 1: %s", n, w.Body.String())
+			}
+			if w.Code != http.StatusForbidden {
+				t.Errorf("status = %d, want 403 (%s)", w.Code, w.Body.String())
+			}
+		})
+	}
+}

src/frontend/apiserver/controllers/integration_test.go

@@ -189,7 +189,10 @@ users:
 
 	r := gin.New()
 	r.POST("/test", func(ctx *gin.Context) {
-		successHosts, failedHosts := validateHost(ctx, 1, []string{"node01", "nonexistent"})
+		successHosts, failedHosts, err := validateHost(ctx, 1, []string{"node01", "nonexistent"})
+		if err != nil {
+			t.Fatalf("validateHost: %v", err)
+		}
 		foundNode01 := false
 		for _, h := range successHosts {
 			if h == "node01" {