OSAC-3609: restrict public filters to public object fields

fulfillment-service/internal/database/dao/dao_errors.go

@@ -148,6 +148,20 @@ func (e *ErrImmutable) Error() string {
 	return fmt.Sprintf("fields %s are immutable", english.WordSeries(quoted, "and"))
 }
 
+// ErrInvalidFilter indicates that a CEL filter expression failed to translate — either a syntax error, a reference
+// to a field that doesn't exist on the type visible to the caller, or an unsupported operation on a field kind. The
+// reason string is derived from the caller's own filter expression and CEL diagnostics, so it is safe to return as
+// part of the error response, for example as the message of a gRPC status error — it must never be built by
+// interpolating internal implementation details, such as generated SQL text, into the message.
+type ErrInvalidFilter struct {
+	Reason string
+}
+
+// Error returns the error message.
+func (e *ErrInvalidFilter) Error() string {
+	return e.Reason
+}
+
 // ErrReference indicates that an operation failed because it references an entity that doesn't exist, for example a
 // tenant or a project.
 type ErrReference struct {

fulfillment-service/internal/database/dao/dao_errors_test.go

@@ -169,6 +169,23 @@ var _ = Describe("Errors", func() {
 		})
 	})
 
+	Describe("ErrInvalidFilter", func() {
+		It("Implements the error interface", func() {
+			var err error = &ErrInvalidFilter{Reason: "field doesn't exist"}
+			Expect(err).To(HaveOccurred())
+		})
+
+		It("Returns the Reason field as the error message", func() {
+			err := &ErrInvalidFilter{Reason: "field 'my_field' doesn't exist"}
+			Expect(err.Error()).To(Equal("field 'my_field' doesn't exist"))
+		})
+
+		It("Returns empty string when Reason is empty", func() {
+			err := &ErrInvalidFilter{}
+			Expect(err.Error()).To(BeEmpty())
+		})
+	})
+
 	Describe("ErrReference", func() {
 		It("Implements the error interface", func() {
 			var err error = &ErrReference{

fulfillment-service/internal/database/dao/filter_translator_test.go

@@ -20,21 +20,52 @@ import (
 	. "github.com/onsi/ginkgo/v2/dsl/table"
 	. "github.com/onsi/gomega"
 
+	"google.golang.org/protobuf/reflect/protoreflect"
+
 	privatev1 "github.com/osac-project/osac/fulfillment-service/internal/api/osac/private/v1"
+	publicv1 "github.com/osac-project/osac/fulfillment-service/internal/api/osac/public/v1"
 	testsv1 "github.com/osac-project/osac/fulfillment-service/internal/api/osac/tests/v1"
 )
 
 var _ = Describe("Filter translator", func() {
+	Describe("Construction", func() {
+		It("Fails without a descriptor", func() {
+			_, err := NewFilterTranslator().
+				SetLogger(logger).
+				Build()
+			Expect(err).To(HaveOccurred())
+		})
+
+		It("Succeeds with a descriptor", func() {
+			_, err := NewFilterTranslator().
+				SetLogger(logger).
+				SetDescriptor((*testsv1.Object)(nil).ProtoReflect().Descriptor()).
+				Build()
+			Expect(err).ToNot(HaveOccurred())
+		})
+	})
+
 	Describe("Object translation", func() {
-		var translator *FilterTranslator[*testsv1.Object]
+		var translator *FilterTranslator
 
 		BeforeEach(func() {
 			var err error
 
-			translator, err = NewFilterTranslator[*testsv1.Object]().
+			translator, err = NewFilterTranslator().
+				SetLogger(logger).
+				SetDescriptor((*testsv1.Object)(nil).ProtoReflect().Descriptor()).
+				Build()
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		It("Rejects a filter referencing a field absent from the configured descriptor", func(ctx context.Context) {
+			restricted, err := NewFilterTranslator().
 				SetLogger(logger).
+				SetDescriptor((*testsv1.Status)(nil).ProtoReflect().Descriptor()).
 				Build()
 			Expect(err).ToNot(HaveOccurred())
+			_, err = restricted.Translate(ctx, "this.my_bool")
+			Expect(err).To(HaveOccurred())
 		})
 
 		DescribeTable(
@@ -374,6 +405,16 @@ var _ = Describe("Filter translator", func() {
 				`this.metadata.project.contains('my')`,
 				`cast(project as text) like '%my%'`,
 			),
+			Entry(
+				"Bracket-index on a map field nested under spec",
+				`this.spec.spec_map["key"] == null`,
+				`data->'spec'->'spec_map'->>'key' is null`,
+			),
+			Entry(
+				"Key presence in a map field nested under spec",
+				`'key' in this.spec.spec_map`,
+				`data->'spec'->'spec_map' ? 'key'`,
+			),
 		)
 
 		DescribeTable(
@@ -395,49 +436,88 @@ var _ = Describe("Filter translator", func() {
 				`this.spec.spec_enum != (1 + 1)`,
 			),
 		)
-	})
-
-	// Projects need special translation because the type of the 'name' column is 'ltree', and that can't be
-	// compared directly to strings using the 'like' operator.
-	Describe("Project translation", func() {
-		var translator *FilterTranslator[*privatev1.Project]
-
-		BeforeEach(func() {
-			var err error
-
-			translator, err = NewFilterTranslator[*privatev1.Project]().
-				SetLogger(logger).
-				Build()
-			Expect(err).ToNot(HaveOccurred())
-		})
 
+		// These two cases are currently rejected by CEL's own type checker at Compile() time — the 'in' operator
+		// has no overload for a plain string/message operand — so they don't exercise translateInField's
+		// default branch directly. They're kept as regression tests of the externally observable contract
+		// (translate-time error, never broken SQL for an unsupported 'in' target), which also covers that
+		// branch if CEL's checking behavior ever changes.
 		DescribeTable(
-			"Project translation",
-			func(ctx context.Context, filter, expected string) {
-				actual, err := translator.Translate(ctx, filter)
-				Expect(err).ToNot(HaveOccurred())
-				Expect(actual).To(Equal(expected))
+			"'in' operator: unsupported target kind errors",
+			func(ctx context.Context, filter string) {
+				_, err := translator.Translate(ctx, filter)
+				Expect(err).To(HaveOccurred())
 			},
 			Entry(
-				"Compare name to string",
-				`this.metadata.name == 'my_project'`,
-				`name = 'my_project'`,
+				"'in' operator against a plain string field",
+				`'x' in this.spec.spec_string`,
 			),
 			Entry(
-				"Name starts with string",
-				`this.metadata.name.startsWith('my_project.')`,
-				`cast(name as text) like 'my\_project.%'`,
-			),
-			Entry(
-				"Name ends with string",
-				`this.metadata.name.endsWith('.my_project')`,
-				`cast(name as text) like '%.my\_project'`,
-			),
-			Entry(
-				"Name contains string",
-				`this.metadata.name.contains('my')`,
-				`cast(name as text) like '%my%'`,
+				"'in' operator against a nested message field",
+				`'x' in this.spec.spec_msg`,
 			),
 		)
+
+		It("Does not leak the JSON operand path in the unsupported-field-kind error", func(ctx context.Context) {
+			_, err := translator.Translate(ctx, `this.spec.spec_bytes == this.spec.spec_bytes`)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(Equal(
+				"select of JSON field 'spec_bytes' of type 'osac.tests.v1.Spec' of kind 'bytes' isn't supported",
+			))
+		})
+	})
+
+	// Projects need special translation because the type of the 'name' column is 'ltree', and that can't be
+	// compared directly to strings using the 'like' operator. Both the public and private Project descriptors
+	// must be recognized, since a public-fronted ProjectsServer now configures the translator with the public
+	// descriptor while PrivateProjectsServer keeps using the private one.
+	Describe("Project translation", func() {
+		for _, desc := range []protoreflect.MessageDescriptor{
+			(*publicv1.Project)(nil).ProtoReflect().Descriptor(),
+			(*privatev1.Project)(nil).ProtoReflect().Descriptor(),
+		} {
+			Describe(string(desc.FullName()), func() {
+				var translator *FilterTranslator
+
+				BeforeEach(func() {
+					var err error
+
+					translator, err = NewFilterTranslator().
+						SetLogger(logger).
+						SetDescriptor(desc).
+						Build()
+					Expect(err).ToNot(HaveOccurred())
+				})
+
+				DescribeTable(
+					"Project translation",
+					func(ctx context.Context, filter, expected string) {
+						actual, err := translator.Translate(ctx, filter)
+						Expect(err).ToNot(HaveOccurred())
+						Expect(actual).To(Equal(expected))
+					},
+					Entry(
+						"Compare name to string",
+						`this.metadata.name == 'my_project'`,
+						`name = 'my_project'`,
+					),
+					Entry(
+						"Name starts with string",
+						`this.metadata.name.startsWith('my_project.')`,
+						`cast(name as text) like 'my\_project.%'`,
+					),
+					Entry(
+						"Name ends with string",
+						`this.metadata.name.endsWith('.my_project')`,
+						`cast(name as text) like '%.my\_project'`,
+					),
+					Entry(
+						"Name contains string",
+						`this.metadata.name.contains('my')`,
+						`cast(name as text) like '%my%'`,
+					),
+				)
+			})
+		}
 	})
 })

fulfillment-service/internal/database/dao/generic_dao.go

@@ -46,6 +46,7 @@ type GenericDAOBuilder[O Object] struct {
 	eventCallbacks    []EventCallback
 	tenancyLogic      auth.TenancyLogic
 	metricsRegisterer prometheus.Registerer
+	filterDesc        protoreflect.MessageDescriptor
 }
 
 // GenericDAO provides generic data access operations for protocol buffers messages. It assumes that objects will be
@@ -79,7 +80,7 @@ type GenericDAO[O Object] struct {
 	jsonEncoder      *json.Encoder
 	marshalOptions   protojson.MarshalOptions
 	unmarshalOptions protojson.UnmarshalOptions
-	filterTranslator *FilterTranslator[O]
+	filterTranslator *FilterTranslator
 	tenancyLogic     auth.TenancyLogic
 
 	// Metrics:
@@ -189,6 +190,14 @@ func (b *GenericDAOBuilder[O]) SetTableName(value string) *GenericDAOBuilder[O]
 	return b
 }
 
+// SetFilterDesc sets the protobuf message descriptor used to validate and translate CEL filter expressions. This is
+// optional. When unset, the descriptor of the O generic parameter is used. Pass a different descriptor to restrict
+// which fields clients may reference in filters — public servers over private storage pass the public descriptor.
+func (b *GenericDAOBuilder[O]) SetFilterDesc(value protoreflect.MessageDescriptor) *GenericDAOBuilder[O] {
+	b.filterDesc = value
+	return b
+}
+
 // Build creates a new generic DAO using the configuration stored in the builder.
 func (b *GenericDAOBuilder[O]) Build() (result *GenericDAO[O], err error) {
 	// Check parameters:
@@ -271,9 +280,15 @@ func (b *GenericDAOBuilder[O]) Build() (result *GenericDAO[O], err error) {
 		DiscardUnknown: true,
 	}
 
-	// Create the filter translator:
-	filterTranslator, err := NewFilterTranslator[O]().
+	// Create the filter translator. The filter descriptor defaults to the object's own descriptor, but callers
+	// may override it via SetFilterDesc to restrict which fields are visible to filter expressions.
+	filterDesc := b.filterDesc
+	if filterDesc == nil {
+		filterDesc = objectDesc
+	}
+	filterTranslator, err := NewFilterTranslator().
 		SetLogger(b.logger).
+		SetDescriptor(filterDesc).
 		Build()
 	if err != nil {
 		err = fmt.Errorf("failed to create filter translator: %w", err)

fulfillment-service/internal/database/dao/generic_dao_list.go

@@ -75,6 +75,7 @@ func (r *ListRequest[O]) do(ctx context.Context) (response *ListResponse[O], err
 		var filter string
 		filter, err = r.dao.filterTranslator.Translate(ctx, r.filter)
 		if err != nil {
+			err = &ErrInvalidFilter{Reason: err.Error()}
 			return
 		}
 		if r.sql.filter.Len() > 0 {

fulfillment-service/internal/database/dao/generic_dao_test.go

@@ -2579,6 +2579,49 @@ var _ = Describe("Generic DAO", func() {
 				Expect(object.GetMetadata().GetName()).To(Equal("object-a"))
 			})
 		})
+
+		Describe("Filter descriptor override", func() {
+			It("Behaves identically when SetFilterDesc is set explicitly to the object's own descriptor", func() {
+				explicitDao, err := NewGenericDAO[*testsv1.Object]().
+					SetLogger(logger).
+					SetTenancyLogic(tenancy).
+					SetFilterDesc((*testsv1.Object)(nil).ProtoReflect().Descriptor()).
+					Build()
+				Expect(err).ToNot(HaveOccurred())
+				_, err = explicitDao.Create().
+					SetObject(
+						testsv1.Object_builder{
+							Metadata: testsv1.Metadata_builder{
+								Tenant: "my-tenant",
+								Name:   "my-object",
+							}.Build(),
+							MyBool: true,
+						}.Build(),
+					).
+					Do(ctx)
+				Expect(err).ToNot(HaveOccurred())
+				response, err := explicitDao.List().
+					SetFilter("this.my_bool == true").
+					Do(ctx)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(response.GetItems()).To(HaveLen(1))
+			})
+
+			It("Rejects a filter referencing a field absent from the overridden descriptor", func() {
+				restrictedDao, err := NewGenericDAO[*testsv1.Object]().
+					SetLogger(logger).
+					SetTenancyLogic(tenancy).
+					SetFilterDesc((*testsv1.Status)(nil).ProtoReflect().Descriptor()).
+					Build()
+				Expect(err).ToNot(HaveOccurred())
+				_, err = restrictedDao.List().
+					SetFilter("this.my_bool == true").
+					Do(ctx)
+				Expect(err).To(HaveOccurred())
+				var invalidFilterErr *ErrInvalidFilter
+				Expect(errors.As(err, &invalidFilterErr)).To(BeTrue())
+			})
+		})
 	})
 
 	Describe("Project filtering", func() {

fulfillment-service/internal/servers/bare_metal_instance_types_server.go

@@ -124,6 +124,7 @@ func (b *BareMetalInstanceTypesServerBuilder) Build() (result *BareMetalInstance
 		SetAttributionLogic(b.attributionLogic).
 		SetTenancyLogic(b.tenancyLogic).
 		SetMetricsRegisterer(b.metricsRegisterer).
+		SetFilterDesc((*publicv1.BareMetalInstanceType)(nil).ProtoReflect().Descriptor()).
 		Build()
 	if err != nil {
 		return

fulfillment-service/internal/servers/baremetal_instance_catalog_items_server.go

@@ -126,6 +126,7 @@ func (b *BareMetalInstanceCatalogItemsServerBuilder) Build() (result *BareMetalI
 		SetTenancyLogic(b.tenancyLogic).
 		SetMetricsRegisterer(b.metricsRegisterer).
 		SetReferenceChecker(referenceChecker).
+		SetFilterDesc((*publicv1.BareMetalInstanceCatalogItem)(nil).ProtoReflect().Descriptor()).
 		Build()
 	if err != nil {
 		return

fulfillment-service/internal/servers/baremetal_instance_templates_server.go

@@ -105,6 +105,7 @@ func (b *BareMetalInstanceTemplatesServerBuilder) Build() (result *BareMetalInst
 		SetAttributionLogic(b.attributionLogic).
 		SetTenancyLogic(b.tenancyLogic).
 		SetMetricsRegisterer(b.metricsRegisterer).
+		SetFilterDesc((*publicv1.BareMetalInstanceTemplate)(nil).ProtoReflect().Descriptor()).
 		Build()
 	if err != nil {
 		return

fulfillment-service/internal/servers/baremetal_instances_server.go

@@ -113,6 +113,7 @@ func (b *BareMetalInstancesServerBuilder) Build() (result *BareMetalInstancesSer
 		SetAttributionLogic(b.attributionLogic).
 		SetTenancyLogic(b.tenancyLogic).
 		SetMetricsRegisterer(b.metricsRegisterer).
+		SetFilterDesc((*publicv1.BareMetalInstance)(nil).ProtoReflect().Descriptor()).
 		Build()
 	if err != nil {
 		return

fulfillment-service/internal/servers/cluster_catalog_items_server.go

@@ -119,6 +119,7 @@ func (b *ClusterCatalogItemsServerBuilder) Build() (result *ClusterCatalogItemsS
 		SetAttributionLogic(b.attributionLogic).
 		SetTenancyLogic(b.tenancyLogic).
 		SetMetricsRegisterer(b.metricsRegisterer).
+		SetFilterDesc((*publicv1.ClusterCatalogItem)(nil).ProtoReflect().Descriptor()).
 		Build()
 	if err != nil {
 		return

fulfillment-service/internal/servers/cluster_templates_server.go

@@ -116,6 +116,7 @@ func (b *ClusterTemplatesServerBuilder) Build() (result *ClusterTemplatesServer,
 		SetAttributionLogic(b.attributionLogic).
 		SetTenancyLogic(b.tenancyLogic).
 		SetMetricsRegisterer(b.metricsRegisterer).
+		SetFilterDesc((*publicv1.ClusterTemplate)(nil).ProtoReflect().Descriptor()).
 		Build()
 	if err != nil {
 		return