HYPERFLEET-1375 - fix: validate typed search fields and close DB column leaks

pkg/db/sql_helpers.go

@@ -37,16 +37,6 @@ func validateJSONBKey(key, fieldType string) *errors.ServiceError {
 func getField(name string) (field string, err *errors.ServiceError) {
 	trimmedName := strings.Trim(name, " ")
 
-	if strings.HasPrefix(trimmedName, "properties.") {
-		key := strings.TrimPrefix(trimmedName, "properties.")
-		if validationErr := validateJSONBKey(key, "property key"); validationErr != nil {
-			err = validationErr
-			return
-		}
-		field = fmt.Sprintf("properties ->> '%s'", key)
-		return
-	}
-
 	// Map user-friendly spec.xxx (and nested spec.xxx.yyy...) syntax to JSONB query.
 	// v6 gives us the full dotted path directly:
 	//   spec.region              → spec->>'region'
@@ -575,9 +565,109 @@ func FieldNameWalk(n *tsl.Node) (*tsl.Node, *errors.ServiceError) {
 		}
 		return n, errors.BadRequest("%s", err.Error())
 	}
+	if svcErr := validateTypedFieldValues(mapped); svcErr != nil {
+		return n, svcErr
+	}
 	return wrapSpecNumericCasts(mapped), nil
 }
 
+// searchTypedFields declares the expected TSL literal kind for top-level search
+// columns whose underlying SQL type does not accept arbitrary text (INTEGER,
+// TIMESTAMPTZ). Fields not listed here are VARCHAR and accept any string
+// literal without a Postgres type-mismatch risk.
+var searchTypedFields = map[string]tsl.Kind{
+	"generation":   tsl.KindNumericLiteral,
+	"created_time": tsl.KindTimestampLiteral,
+	"updated_time": tsl.KindTimestampLiteral,
+	"deleted_time": tsl.KindTimestampLiteral,
+}
+
+// typedKindHints gives a human-readable description of each tsl.Kind used in
+// searchTypedFields, for use in validation error messages.
+var typedKindHints = map[tsl.Kind]string{
+	tsl.KindNumericLiteral:   "an integer",
+	tsl.KindTimestampLiteral: "an RFC3339 timestamp (e.g. 2026-01-01T00:00:00Z)",
+}
+
+// validateTypedFieldValues walks binary comparisons against a searchTypedFields
+// column — on either side of the operator, since TSL allows both orders (see
+// wrapSpecNumericCasts below) — and rejects values whose TSL literal kind
+// doesn't match the column's SQL type, before the query reaches Postgres as a
+// raw pq: error.
+func validateTypedFieldValues(n *tsl.Node) *errors.ServiceError {
+	if n == nil {
+		return nil
+	}
+	if n.Kind == tsl.KindBinaryExpr {
+		if err := validateTypedOperand(n.Left, n.Right); err != nil {
+			return err
+		}
+		if err := validateTypedOperand(n.Right, n.Left); err != nil {
+			return err
+		}
+	}
+	if err := validateTypedFieldValues(n.Left); err != nil {
+		return err
+	}
+	if err := validateTypedFieldValues(n.Right); err != nil {
+		return err
+	}
+	for _, child := range n.Children {
+		if err := validateTypedFieldValues(child); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// validateTypedOperand checks whether ident is a tracked typed-field identifier
+// and, if so, validates value against that field's expected TSL literal kind.
+// It's a no-op when ident isn't a tracked identifier (e.g. it's the value side).
+func validateTypedOperand(ident, value *tsl.Node) *errors.ServiceError {
+	if ident == nil || ident.Kind != tsl.KindIdentifier {
+		return nil
+	}
+	field, ok := ident.Value.(string)
+	if !ok {
+		return nil
+	}
+	wantKind, tracked := searchTypedFields[field]
+	if !tracked {
+		return nil
+	}
+	return validateTypedValue(field, wantKind, value)
+}
+
+// validateTypedValue checks a single value node (or, for IN queries, each
+// element of an array literal) against the expected TSL literal kind. A date
+// literal (no time component) is accepted wherever a timestamp is expected.
+// null is always accepted, since typed columns here are nullable (e.g.
+// deleted_time IS NULL / IS NOT NULL is a standard soft-delete filter).
+func validateTypedValue(field string, wantKind tsl.Kind, value *tsl.Node) *errors.ServiceError {
+	if value == nil {
+		return errors.BadRequest("missing value for field '%s'", field)
+	}
+	if value.Kind == tsl.KindArrayLiteral {
+		for _, child := range value.Children {
+			if err := validateTypedValue(field, wantKind, child); err != nil {
+				return err
+			}
+		}
+		return nil
+	}
+	if value.Kind == tsl.KindNullLiteral {
+		return nil
+	}
+	if value.Kind == wantKind || (wantKind == tsl.KindTimestampLiteral && value.Kind == tsl.KindDateLiteral) {
+		return nil
+	}
+	hint, ok := typedKindHints[wantKind]
+	if !ok {
+		hint = fmt.Sprintf("a value of kind %s", wantKind)
+	}
+	return errors.BadRequest("field '%s' expects %s", field, hint)
+}
+
 // wrapSpecNumericCasts wraps spec JSONB fields in CAST(... AS numeric) when
 // compared against a number, so multi-digit values sort correctly.
 func wrapSpecNumericCasts(n *tsl.Node) *tsl.Node {

pkg/db/sql_helpers_test.go

@@ -797,6 +797,120 @@ func TestFieldNameWalk_NumericCast(t *testing.T) {
 	})
 }
 
+// TestFieldNameWalk_TypedFieldValidation verifies that typed top-level columns
+// (generation: INTEGER; created_time/updated_time/deleted_time: TIMESTAMPTZ)
+// reject mismatched values with a clean 400 before ever reaching Postgres.
+func TestFieldNameWalk_TypedFieldValidation(t *testing.T) {
+	tests := []struct {
+		name          string
+		searchQuery   string
+		errorContains string
+		expectError   bool
+	}{
+		{
+			name:        "generation with valid integer",
+			searchQuery: "generation = 5",
+		},
+		{
+			name:          "generation with non-integer string value",
+			searchQuery:   "generation = 'abc'",
+			expectError:   true,
+			errorContains: "field 'generation' expects an integer",
+		},
+		{
+			name:        "created_time with valid RFC3339 timestamp",
+			searchQuery: "created_time > '2026-01-01T00:00:00Z'",
+		},
+		{
+			name:          "created_time with non-timestamp string value",
+			searchQuery:   "created_time = 'not-a-date'",
+			expectError:   true,
+			errorContains: "field 'created_time' expects an RFC3339 timestamp",
+		},
+		{
+			name:          "updated_time with non-timestamp string value",
+			searchQuery:   "updated_time = 'not-a-date'",
+			expectError:   true,
+			errorContains: "field 'updated_time' expects an RFC3339 timestamp",
+		},
+		{
+			name:          "deleted_time with non-timestamp string value",
+			searchQuery:   "deleted_time = 'not-a-date'",
+			expectError:   true,
+			errorContains: "field 'deleted_time' expects an RFC3339 timestamp",
+		},
+		{
+			name:          "generation IN list with a non-integer element",
+			searchQuery:   "generation IN [1, 'abc']",
+			expectError:   true,
+			errorContains: "field 'generation' expects an integer",
+		},
+		{
+			name:        "generation IN list with all integers",
+			searchQuery: "generation IN [1, 2, 3]",
+		},
+		{
+			name:        "combined query: typed field valid, text field untouched",
+			searchQuery: "generation = 1 AND name = 'anything-goes'",
+		},
+		{
+			name:          "combined query: error surfaces even when other predicate is valid",
+			searchQuery:   "name = 'ok' AND generation = 'abc'",
+			expectError:   true,
+			errorContains: "field 'generation' expects an integer",
+		},
+		{
+			name:          "generation with literal-first comparison (field on right)",
+			searchQuery:   "'abc' = generation",
+			expectError:   true,
+			errorContains: "field 'generation' expects an integer",
+		},
+		{
+			name:          "created_time with literal-first comparison (field on right)",
+			searchQuery:   "'not-a-date' < created_time",
+			expectError:   true,
+			errorContains: "field 'created_time' expects an RFC3339 timestamp",
+		},
+		{
+			name:        "generation with literal-first comparison and valid value",
+			searchQuery: "1 < generation",
+		},
+		{
+			name:        "deleted_time IS NULL is accepted (soft-delete filter)",
+			searchQuery: "deleted_time IS NULL",
+		},
+		{
+			name:        "deleted_time IS NOT NULL is accepted (soft-delete filter)",
+			searchQuery: "deleted_time IS NOT NULL",
+		},
+		{
+			name:        "created_time with valid date literal (no time component)",
+			searchQuery: "created_time > '2026-01-01'",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			RegisterTestingT(t)
+
+			tree, err := tsl.ParseTSL(tt.searchQuery)
+			Expect(err).ToNot(HaveOccurred())
+
+			_, serviceErr := FieldNameWalk(tree.Node)
+
+			if tt.expectError {
+				Expect(serviceErr).ToNot(BeNil())
+				Expect(serviceErr.Error()).To(ContainSubstring(tt.errorContains))
+				// No raw driver details should ever leak into the message.
+				Expect(serviceErr.Error()).ToNot(ContainSubstring("pq:"))
+				Expect(serviceErr.Error()).ToNot(ContainSubstring("SQLSTATE"))
+				return
+			}
+			Expect(serviceErr).To(BeNil())
+		})
+	}
+}
+
 func TestConditionStatusValidation(t *testing.T) {
 	tests := []struct {
 		status      string

pkg/handlers/validation.go

@@ -8,13 +8,18 @@ import (
 	"time"
 	"unicode/utf8"
 
+	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/api"
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi"
 	"github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors"
 )
 
 const (
-	maxReasonLength     = 1024
-	maxLabelKeyLen      = 317 // 253-char prefix + "/" + 63-char name
+	maxReasonLength = 1024
+	// The Kubernetes label key spec allows up to 317 chars (253-char prefix + "/" + 63-char
+	// name), but resource_labels.key is VARCHAR(255) — cap at the DB limit so an
+	// oversized-but-K8s-valid key fails here with a clean 400 instead of during resource
+	// conversion with a 500 (api.ValidateLabel / api.MaxLabelKeyLen enforce the same bound).
+	maxLabelKeyLen      = api.MaxLabelKeyLen
 	maxLabelValueLen    = 63
 	maxObservedTimeSkew = 5 * time.Minute  // tolerance for clock skew between adapter pods and API server
 	maxObservedTimeAge  = 30 * time.Minute // matches Sentinel staleness health check window

pkg/handlers/validation_test.go

@@ -639,6 +639,27 @@ func TestValidateLabels_InvalidValues(t *testing.T) {
 	}
 }
 
+// TestValidateLabels_KeyExceedsDBColumnLimit exercises the maxLabelKeyLen bound
+// (see its doc comment) with a key that is K8s-spec-valid but DB-column-oversized.
+func TestValidateLabels_KeyExceedsDBColumnLimit(t *testing.T) {
+	RegisterTestingT(t)
+
+	seg := strings.Repeat("a", 62)
+	prefix := strings.Join([]string{seg, seg, seg, seg}, ".") // 4*62 + 3 = 251 chars, each segment valid DNS-1123
+	name := strings.Repeat("b", 63)
+	key := prefix + "/" + name // 251 + 1 + 63 = 315 chars: valid K8s key, exceeds VARCHAR(255)
+	Expect(len(key)).To(Equal(315))
+	Expect(isValidLabelKeyPrefix(prefix)).To(BeTrue(), "prefix must be spec-valid for this test to be meaningful")
+
+	labels := map[string]string{key: "valid-value"}
+	req := openapi.ClusterCreateRequest{Labels: &labels}
+	validator := validateLabels(&req, "Labels")
+	err := validator()
+
+	Expect(err).ToNot(BeNil(), "expected an oversized-but-K8s-valid key to fail validation")
+	Expect(err.Reason).To(ContainSubstring("label validation failed"))
+}
+
 func TestValidateLabels_NilLabels(t *testing.T) {
 	RegisterTestingT(t)
 

pkg/services/generic_test.go

@@ -33,6 +33,14 @@ 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
@@ -132,11 +140,6 @@ func TestSQLTranslation(t *testing.T) {
 			"sql":    "spec->'release'->>'version' = ?",
 			"values": ConsistOf("2"),
 		},
-		{
-			"search": "properties.owner = 'team_a'",
-			"sql":    "properties ->> 'owner' = ?",
-			"values": ConsistOf("team_a"),
-		},
 	}
 	for _, test := range jsonbRelatedTableTests {
 		var list []api.Resource