HYPERFLEET-1376 - fix: avoid leaking internal type names in validation errors

pkg/handlers/framework.go

@@ -3,8 +3,10 @@ package handlers
 import (
 	"bytes"
 	"encoding/json"
+	goerrors "errors"
 	"io"
 	"net/http"
+	"reflect"
 
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/presenters"
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/response"
@@ -85,11 +87,19 @@ func handle(w http.ResponseWriter, r *http.Request, cfg *handlerConfig, httpStat
 		dec := json.NewDecoder(bytes.NewReader(bodyBytes))
 		dec.DisallowUnknownFields()
 		if err := dec.Decode(cfg.MarshalInto); err != nil {
+			if typeErr := cleanTypeMismatchError(err); typeErr != nil {
+				handleError(r, w, typeErr)
+				return
+			}
 			handleError(r, w, errors.BadRequest("Unknown or immutable field in request body: %s", err))
 			return
 		}
 	} else {
 		if err := json.Unmarshal(bodyBytes, &cfg.MarshalInto); err != nil {
+			if typeErr := cleanTypeMismatchError(err); typeErr != nil {
+				handleError(r, w, typeErr)
+				return
+			}
 			handleError(r, w, errors.MalformedRequest("Invalid request format: %s", err))
 			return
 		}
@@ -138,6 +148,35 @@ func handleSoftDelete(w http.ResponseWriter, r *http.Request, cfg *handlerConfig
 
 }
 
+func cleanTypeMismatchError(err error) *errors.ServiceError {
+	var typeErr *json.UnmarshalTypeError
+	if !goerrors.As(err, &typeErr) {
+		return nil
+	}
+	return errors.Validation("field '%s' must be %s", typeErr.Field, describeJSONKind(typeErr.Type.Kind()))
+}
+
+func describeJSONKind(k reflect.Kind) string {
+	switch k {
+	case reflect.Map, reflect.Struct:
+		return "an object"
+	case reflect.Slice, reflect.Array:
+		return "an array"
+	case reflect.String:
+		return "a string"
+	case reflect.Bool:
+		return "a boolean"
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
+		reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64,
+		reflect.Float32, reflect.Float64:
+		return "a number"
+	default:
+		// Fallback for kinds json.UnmarshalTypeError.Type should never report
+		// here (e.g. Ptr, Interface) — avoids mislabeling as a number.
+		return "the correct type"
+	}
+}
+
 func handleGet(w http.ResponseWriter, r *http.Request, cfg *handlerConfig) {
 	if cfg.ErrorHandler == nil {
 		cfg.ErrorHandler = handleError
@@ -180,6 +219,10 @@ func handleForceDelete(w http.ResponseWriter, r *http.Request, cfg *handlerConfi
 
 	err = json.Unmarshal(bytes, &cfg.MarshalInto)
 	if err != nil {
+		if typeErr := cleanTypeMismatchError(err); typeErr != nil {
+			handleError(r, w, typeErr)
+			return
+		}
 		handleError(r, w, errors.MalformedRequest("Invalid request format: %s", err))
 		return
 	}
@@ -217,6 +260,10 @@ func handleCreateWithNoContent(w http.ResponseWriter, r *http.Request, cfg *hand
 
 	err = json.Unmarshal(bytes, &cfg.MarshalInto)
 	if err != nil {
+		if typeErr := cleanTypeMismatchError(err); typeErr != nil {
+			handleError(r, w, typeErr)
+			return
+		}
 		handleError(r, w, errors.MalformedRequest("Invalid request format: %s", err))
 		return
 	}

pkg/handlers/resource_handler_test.go

@@ -276,6 +276,25 @@ func TestResourceHandler_Patch(t *testing.T) {
 			},
 			expectedStatusCode: http.StatusOK,
 		},
+		{
+			// References-only patch must be accepted, not rejected as "no field provided" — HYPERFLEET-1376 finding 8.
+			name: "Success - references only",
+			id:   "ch-123",
+			body: `{"references":{"parents":[{"kind":"Cluster","id":"cluster-1"}]}}`,
+			setupMock: func(mock *services.MockResourceService) {
+				mock.EXPECT().Patch(gomock.Any(), "Channel", "ch-123", gomock.AssignableToTypeOf(&api.ResourcePatch{})).
+					Return(&api.Resource{
+						Meta:       api.Meta{ID: "ch-123", CreatedTime: now, UpdatedTime: now},
+						Kind:       "Channel",
+						Name:       "stable",
+						Spec:       datatypes.JSON(`{"is_default":false}`),
+						Generation: 2,
+						CreatedBy:  "user@test.com",
+						UpdatedBy:  "user@test.com",
+					}, nil)
+			},
+			expectedStatusCode: http.StatusOK,
+		},
 		{
 			name: "Not found",
 			id:   "nonexistent",
@@ -728,6 +747,15 @@ func TestResourceHandler_ForceDelete(t *testing.T) {
 			},
 			expectedStatusCode: http.StatusBadRequest,
 		},
+		{
+			// Wrong JSON type for "reason" must return a clean message without leaking
+			// the Go struct name (ForceDeleteRequest) — HYPERFLEET-1376.
+			name: "Error 400 - reason wrong type",
+			body: `{"reason": 123}`,
+			setupMock: func(mock *services.MockResourceService) {
+			},
+			expectedStatusCode: http.StatusBadRequest,
+		},
 		{
 			name: "Error 400 - empty reason",
 			body: `{"reason": ""}`,
@@ -797,6 +825,10 @@ func TestResourceHandler_ForceDelete(t *testing.T) {
 			if tt.expectedStatusCode == http.StatusNoContent {
 				Expect(rr.Body.Len()).To(Equal(0))
 			}
+			if tt.name == "Error 400 - reason wrong type" {
+				Expect(rr.Body.String()).To(ContainSubstring("field 'reason' must be a string"))
+				Expect(rr.Body.String()).ToNot(ContainSubstring("ForceDeleteRequest"))
+			}
 		})
 	}
 }
@@ -933,6 +965,46 @@ func TestResourceHandler_Patch_RejectsUnknownFields(t *testing.T) {
 	}
 }
 
+// TestResourceHandler_Patch_LabelsTypeMismatch verifies that a wrong JSON type for
+// "labels" returns a clean validation message without leaking the Go struct name
+// (e.g. "ResourcePatchRequest") or type (e.g. "map[string]string") — HYPERFLEET-1376
+// findings 5 & 6.
+func TestResourceHandler_Patch_LabelsTypeMismatch(t *testing.T) {
+	RegisterTestingT(t)
+
+	tests := []struct {
+		name string
+		body string
+	}{
+		{"labels as string", `{"labels":"not-an-object"}`},
+		{"labels as array", `{"labels":["a","b"]}`},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			RegisterTestingT(t)
+
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			handler, _ := newTestResourceHandler(ctrl)
+
+			reqURL := "/api/hyperfleet/v1/channels/ch-123"
+			req := httptest.NewRequest(http.MethodPatch, reqURL, strings.NewReader(tt.body))
+			req.Header.Set("Content-Type", "application/json")
+			req = mux.SetURLVars(req, map[string]string{"id": "ch-123"})
+
+			rr := httptest.NewRecorder()
+			handler.Patch(rr, req)
+
+			Expect(rr.Code).To(Equal(http.StatusBadRequest))
+			Expect(rr.Body.String()).To(ContainSubstring("field 'labels' must be an object"))
+			Expect(rr.Body.String()).ToNot(ContainSubstring("ResourcePatchRequest"))
+			Expect(rr.Body.String()).ToNot(ContainSubstring("openapi."))
+		})
+	}
+}
+
 func TestResourceHandler_PatchByOwner_RejectsUnknownFields(t *testing.T) {
 	RegisterTestingT(t)
 

pkg/handlers/resource_status_handler_test.go

@@ -194,6 +194,31 @@ func TestResourceStatusHandler_Create_MissingAdapter_Returns400(t *testing.T) {
 	Expect(w.Code).To(Equal(http.StatusBadRequest))
 }
 
+// TestResourceStatusHandler_Create_ConditionsTypeMismatch verifies that a wrong JSON
+// type for "conditions" returns a clean validation message without leaking the Go
+// struct/package name (e.g. "AdapterStatusCreateRequest", "openapi.ConditionRequest")
+// — HYPERFLEET-1376 finding 7.
+func TestResourceStatusHandler_Create_ConditionsTypeMismatch(t *testing.T) {
+	RegisterTestingT(t)
+	ctrl := gomock.NewController(t)
+
+	handler, _, _ := newTestResourceStatusHandler(ctrl)
+
+	body := `{"adapter":"x","conditions":"not-an-array"}`
+
+	r := httptest.NewRequest(http.MethodPut, "/channels/ch-1/statuses", strings.NewReader(body))
+	r.Header.Set("Content-Type", "application/json")
+	r = mux.SetURLVars(r, map[string]string{"id": testChannelID})
+	w := httptest.NewRecorder()
+
+	handler.Create(w, r)
+
+	Expect(w.Code).To(Equal(http.StatusBadRequest))
+	Expect(w.Body.String()).To(ContainSubstring("field 'conditions' must be an array"))
+	Expect(w.Body.String()).ToNot(ContainSubstring("AdapterStatusCreateRequest"))
+	Expect(w.Body.String()).ToNot(ContainSubstring("openapi."))
+}
+
 func TestResourceStatusHandler_Create_ResourceNotFound(t *testing.T) {
 	RegisterTestingT(t)
 	ctrl := gomock.NewController(t)

pkg/handlers/validation.go

@@ -247,18 +247,20 @@ func validateLabels(i interface{}, fieldName string) validate {
 	}
 }
 
-// validatePatchRequest validates that at least one patchable field (Spec or Labels) is provided
+// validatePatchRequest validates that at least one patchable field (Spec, Labels, or References) is provided
 func validatePatchRequest(i interface{}) validate {
 	return func() *errors.ServiceError {
 		v := reflect.ValueOf(i).Elem()
 
 		spec := v.FieldByName("Spec")
 		labels := v.FieldByName("Labels")
+		references := v.FieldByName("References")
 
 		specPresent := spec.IsValid() && !spec.IsNil()
 		labelsPresent := labels.IsValid() && !labels.IsNil()
+		referencesPresent := references.IsValid() && !references.IsNil()
 
-		if !specPresent && !labelsPresent {
+		if !specPresent && !labelsPresent && !referencesPresent {
 			return errors.BadRequest("at least one field must be provided for update")
 		}
 		return nil

pkg/handlers/validation_test.go

@@ -401,6 +401,56 @@ func TestValidatePatchRequest(t *testing.T) {
 	}
 }
 
+// TestValidatePatchRequest_References uses openapi.ResourcePatchRequest (rather than
+// ClusterPatchRequest, which has no References field) to verify a references-only
+// patch is accepted — see HYPERFLEET-1376 finding 8.
+func TestValidatePatchRequest_References(t *testing.T) {
+	references := map[string][]openapi.ObjectReference{
+		"parents": {{Kind: "Cluster", Id: strPtr("cluster-1")}},
+	}
+	labels := map[string]string{"env": "prod"}
+
+	testCases := []struct {
+		req         openapi.ResourcePatchRequest
+		name        string
+		expectError bool
+	}{
+		{
+			name:        "references only is valid",
+			req:         openapi.ResourcePatchRequest{References: &references},
+			expectError: false,
+		},
+		{
+			name:        "all nil returns error",
+			req:         openapi.ResourcePatchRequest{},
+			expectError: true,
+		},
+		{
+			name:        "references and labels is valid",
+			req:         openapi.ResourcePatchRequest{References: &references, Labels: &labels},
+			expectError: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			RegisterTestingT(t)
+			validator := validatePatchRequest(&tc.req)
+			err := validator()
+			if tc.expectError {
+				Expect(err).ToNot(BeNil())
+				Expect(err.Reason).To(ContainSubstring("at least one field must be provided for update"))
+			} else {
+				Expect(err).To(BeNil())
+			}
+		})
+	}
+}
+
+func strPtr(s string) *string {
+	return &s
+}
+
 func TestValidateObservedGeneration(t *testing.T) {
 	testCases := []struct {
 		name        string