HYPERFLEET-1366 - refactor: remove search and order field allowlists

pkg/db/sql_helpers.go

@@ -69,34 +69,10 @@ func getField(name string) (field string, err *errors.ServiceError) {
 		return
 	}
 
-	baseName := fieldParts[0]
-	if !searchAllowedFields[baseName] {
-		err = errors.BadRequest("%s is not a valid field name", name)
-		return
-	}
-
 	field = trimmedName
 	return
 }
 
-var searchAllowedFields = map[string]bool{
-	"id":           true,
-	"name":         true,
-	"kind":         true,
-	"created_time": true,
-	"updated_time": true,
-	"deleted_time": true,
-	"created_by":   true,
-	"updated_by":   true,
-	"deleted_by":   true,
-	"generation":   true,
-	"href":         true,
-	"labels":       true,
-	"conditions":   true,
-	"owner_id":     true,
-	"owner_kind":   true,
-}
-
 // Condition type validation pattern: PascalCase condition types (e.g., Reconciled, Available, Progressing)
 var conditionTypePattern = regexp.MustCompile(`^[A-Z][a-zA-Z0-9]*$`)
 
@@ -751,27 +727,11 @@ func IdentWalk(n *tsl.Node, check func(string) (string, error)) (*tsl.Node, erro
 	}
 }
 
-// orderAllowedFields defines the whitelist of fields that are allowed to be ordered.
-// This prevents SQL injection and restricts invalid order queries.
-var orderAllowedFields = map[string]bool{
-	"id":           true,
-	"name":         true,
-	"created_time": true,
-	"updated_time": true,
-	"deleted_time": true,
-	"kind":         true,
-	"created_by":   true,
-	"updated_by":   true,
-	"deleted_by":   true,
-	"generation":   true,
-	"href":         true,
-}
-
 // orderPattern matches valid order syntax: field name (letters, digits, underscore) followed by optional asc/desc.
 // This regex rejects SQL injection attempts (semicolons, parentheses, dashes, comments, etc).
 var orderPattern = regexp.MustCompile(`^[a-z_][a-z_]*(\s+(asc|desc))?$`)
 
-// ArgsToOrder validates and cleans order arguments against the allowed fields whitelist.
+// ArgsToOrder validates and cleans order arguments.
 // Returns a cleaned list of order clauses in the format ["field direction", ...]
 // Empty or whitespace-only strings are silently skipped.
 func ArgsToOrder(args []string) (cleanedOrderList []string, err *errors.ServiceError) {
@@ -809,11 +769,6 @@ func ArgsToOrder(args []string) (cleanedOrderList []string, err *errors.ServiceE
 			return nil, errors.BadRequest("invalid order format '%s': expected 'field' or 'field asc|desc'", val)
 		}
 
-		// Validate field against orderAllowedFields
-		if !orderAllowedFields[field] {
-			return nil, errors.BadRequest("field '%s' is not allowed for ordering", field)
-		}
-
 		cleanedValue := fmt.Sprintf("%s %s", field, direction)
 		cleanedOrderList = append(cleanedOrderList, cleanedValue)
 	}

pkg/db/sql_helpers_test.go

@@ -1030,18 +1030,6 @@ func TestArgsToOrder(t *testing.T) {
 			input:    []string{"name asc", "", "created_time desc", "   ", "\t"},
 			expected: []string{"name asc", "created_time desc"},
 		},
-		{
-			name:          "mixed valid and invalid field",
-			input:         []string{"created_time desc", "name", "wrong_field"},
-			expectError:   true,
-			errorContains: "not allowed for ordering",
-		},
-		{
-			name:          "field not in whitelist",
-			input:         []string{"custom_field asc"},
-			expectError:   true,
-			errorContains: "not allowed for ordering",
-		},
 		{
 			name:     "deleted_time field",
 			input:    []string{"deleted_time desc"},

pkg/db/transaction_middleware.go

@@ -8,6 +8,8 @@ import (
 	"net/http"
 	"time"
 
+	"github.com/lib/pq"
+
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/response"
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors"
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger"
@@ -74,6 +76,13 @@ func TransactionMiddleware(next http.Handler, connection SessionFactory, request
 	})
 }
 
+// IsUndefinedColumnError reports whether err is "undefined_column" error,
+// this occurs when a query references a column that does not exist in the schema
+func IsUndefinedColumnError(err error) bool {
+	var pqErr *pq.Error
+	return stderrors.As(err, &pqErr) && pqErr.Code == "42703"
+}
+
 // IsDBConnectionError indicates whether err is an infrastructure failure
 // (network unreachable, connection refused, connection dropped)
 func IsDBConnectionError(err error) bool {

pkg/services/generic.go

@@ -278,6 +278,11 @@ func (s *sqlGenericService) loadList(listCtx *listContext, d dao.GenericDao) *er
 			listCtx.pagingMeta.Size = 0
 		case db.IsDBConnectionError(err):
 			return errors.ServiceUnavailable("Database connection unavailable")
+		case db.IsUndefinedColumnError(err):
+			if listCtx.args.Search != "" || len(listCtx.args.Order) > 0 {
+				return errors.BadRequest("invalid field in search or order query")
+			}
+			return errors.GeneralError("Unable to list resources: %s", err)
 		default:
 			return errors.GeneralError("Unable to list resources: %s", err)
 		}

pkg/services/generic_test.go

@@ -33,14 +33,6 @@ func TestSQLTranslation(t *testing.T) {
 			"search": "= = =",
 			"error":  errors.CodeBadRequest + ": Failed to parse search query: = = =",
 		},
-		// "properties." was a leftover alias from before the resources table's JSONB
-		// column was renamed to "spec" — there is no "properties" column, so this must
-		// be rejected as an unknown field rather than translated into a query against a
-		// nonexistent column (which would surface as a raw pq: error at execution time).
-		{
-			"search": "properties.owner = 'team_a'",
-			"error":  errors.CodeBadRequest + ": properties.owner is not a valid field name",
-		},
 	}
 	for _, test := range errorTests {
 		var list []api.Resource

test/integration/order_field_mapping_test.go

@@ -150,9 +150,9 @@ func TestOrderFieldValidation(t *testing.T) {
 		expectedError string
 	}{
 		{
-			name:          "InvalidFieldName",
+			name:          "NonexistentField",
 			order:         "nonexistent_field asc",
-			expectedError: "not allowed for ordering",
+			expectedError: "invalid field in search or order query",
 		},
 		{
 			name:          "InvalidDirection",
@@ -203,7 +203,7 @@ func TestOrderFieldValidation(t *testing.T) {
 	}
 }
 
-// TestOrderAllowedFields verifies that all whitelisted fields work correctly
+// TestOrderAllowedFields verifies that standard fields work correctly for ordering
 func TestOrderAllowedFields(t *testing.T) {
 	RegisterTestingT(t)
 	h, client := test.RegisterIntegration(t)

test/integration/search_field_mapping_test.go

@@ -1070,3 +1070,23 @@ func TestSearchNodePoolConditionSubfieldLastUpdatedTime(t *testing.T) {
 	}
 	Expect(foundStale).To(BeTrue(), "Expected to find the stale node pool")
 }
+
+// TestSearchNonexistentField verifies that searching on a field that does not exist
+// in the database schema returns a 400 Bad Request
+func TestSearchNonexistentField(t *testing.T) {
+	RegisterTestingT(t)
+	h, client := test.RegisterIntegration(t)
+
+	account := h.NewRandAccount()
+	ctx := h.NewAuthenticatedContext(account)
+
+	search := openapi.SearchParams("nonexistent_column = 'foo'")
+	params := &openapi.GetClustersParams{
+		Search: &search,
+	}
+	resp, err := client.GetClustersWithResponse(ctx, params, test.WithAuthToken(ctx))
+
+	Expect(err).NotTo(HaveOccurred())
+	Expect(resp.StatusCode()).To(Equal(http.StatusBadRequest))
+	Expect(string(resp.Body)).To(ContainSubstring("invalid field in search or order query"))
+}