OSAC-58: filter unpublished catalog items from public API

internal/servers/catalog_item_reference_checker.go

@@ -0,0 +1,46 @@
+/*
+Copyright (c) 2025 Red Hat Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the
+License. You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific
+language governing permissions and limitations under the License.
+*/
+
+package servers
+
+import (
+	"context"
+	"fmt"
+
+	grpccodes "google.golang.org/grpc/codes"
+	grpcstatus "google.golang.org/grpc/status"
+
+	"github.com/osac-project/fulfillment-service/internal/database/dao"
+)
+
+// catalogItemReferenceChecker checks whether the caller has any resources that reference a catalog item.
+type catalogItemReferenceChecker interface {
+	hasReference(ctx context.Context, catalogItemID string) (bool, error)
+}
+
+// daoReferenceChecker implements catalogItemReferenceChecker using a GenericDAO.
+type daoReferenceChecker[Resource dao.Object] struct {
+	resourceDao *dao.GenericDAO[Resource]
+}
+
+func (c *daoReferenceChecker[Resource]) hasReference(ctx context.Context, catalogItemID string) (bool, error) {
+	filter := fmt.Sprintf("this.spec.catalog_item == %q", catalogItemID)
+	response, err := c.resourceDao.List().
+		SetFilter(filter).
+		SetLimit(1).
+		Do(ctx)
+	if err != nil {
+		return false, grpcstatus.Errorf(grpccodes.Internal, "failed to check resource references")
+	}
+	return response.GetTotal() > 0, nil
+}

internal/servers/catalog_item_reference_checker_mock.go

@@ -0,0 +1,58 @@
+/*
+Copyright (c) 2025 Red Hat Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the
+License. You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific
+language governing permissions and limitations under the License.
+*/
+
+package servers
+
+import (
+	"context"
+	"reflect"
+
+	"go.uber.org/mock/gomock"
+)
+
+type MockCatalogItemReferenceChecker struct {
+	ctrl     *gomock.Controller
+	recorder *MockCatalogItemReferenceCheckerMockRecorder
+	isgomock struct{}
+}
+
+type MockCatalogItemReferenceCheckerMockRecorder struct {
+	mock *MockCatalogItemReferenceChecker
+}
+
+func NewMockCatalogItemReferenceChecker(ctrl *gomock.Controller) *MockCatalogItemReferenceChecker {
+	mock := &MockCatalogItemReferenceChecker{ctrl: ctrl}
+	mock.recorder = &MockCatalogItemReferenceCheckerMockRecorder{mock}
+	return mock
+}
+
+func (m *MockCatalogItemReferenceChecker) EXPECT() *MockCatalogItemReferenceCheckerMockRecorder {
+	return m.recorder
+}
+
+func (m *MockCatalogItemReferenceChecker) hasReference(ctx context.Context, catalogItemID string) (bool, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "hasReference", ctx, catalogItemID)
+	ret0, _ := ret[0].(bool)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+func (mr *MockCatalogItemReferenceCheckerMockRecorder) hasReference(ctx, catalogItemID any) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(
+		mr.mock, "hasReference",
+		reflect.TypeOf((*MockCatalogItemReferenceChecker)(nil).hasReference),
+		ctx, catalogItemID,
+	)
+}

internal/servers/catalog_item_validation.go

@@ -15,8 +15,11 @@ package servers
 
 import (
 	"encoding/json"
+	"fmt"
 	"strings"
+	"sync"
 
+	"github.com/google/cel-go/cel"
 	"github.com/santhosh-tekuri/jsonschema/v6"
 	grpccodes "google.golang.org/grpc/codes"
 	grpcstatus "google.golang.org/grpc/status"
@@ -27,6 +30,29 @@ import (
 	privatev1 "github.com/osac-project/fulfillment-service/internal/api/osac/private/v1"
 )
 
+var (
+	celSyntaxEnv     *cel.Env
+	celSyntaxEnvOnce sync.Once
+	celSyntaxEnvErr  error
+)
+
+// validateCELSyntax checks that a filter string is a syntactically valid, complete CEL expression.
+// This prevents filter bypass attacks where a malicious filter like "true) || (true" could
+// break out of parenthesized composition and change operator precedence.
+func validateCELSyntax(filter string) error {
+	celSyntaxEnvOnce.Do(func() {
+		celSyntaxEnv, celSyntaxEnvErr = cel.NewEnv()
+	})
+	if celSyntaxEnvErr != nil {
+		return fmt.Errorf("failed to create CEL environment: %w", celSyntaxEnvErr)
+	}
+	_, issues := celSyntaxEnv.Parse(filter)
+	if issues != nil && issues.Err() != nil {
+		return fmt.Errorf("syntax error: %w", issues.Err())
+	}
+	return nil
+}
+
 // catalogItem is implemented by both ClusterCatalogItem and ComputeInstanceCatalogItem.
 type catalogItem interface {
 	proto.Message

internal/servers/catalog_item_validation_test.go

@@ -0,0 +1,73 @@
+/*
+Copyright (c) 2025 Red Hat Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the
+License. You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific
+language governing permissions and limitations under the License.
+*/
+
+package servers
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+var _ = Describe("addPublishedFilter", func() {
+	var server *ClusterCatalogItemsServer
+
+	BeforeEach(func() {
+		server = &ClusterCatalogItemsServer{}
+	})
+
+	DescribeTable("composes filter correctly",
+		func(input string, expected string) {
+			result, err := server.addPublishedFilter(input)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(result).To(Equal(expected))
+		},
+		Entry("empty filter", "", "this.published"),
+		Entry("simple filter", "this.id == '123'", "(this.id == '123') && this.published"),
+		Entry("compound filter", "this.title == 'a' && this.template == 'b'",
+			"(this.title == 'a' && this.template == 'b') && this.published"),
+		Entry("valid filter with OR is safely composed", "true || true",
+			"(true || true) && this.published"),
+	)
+
+	DescribeTable("rejects malformed filters",
+		func(input string) {
+			_, err := server.addPublishedFilter(input)
+			Expect(err).To(HaveOccurred())
+			Expect(status.Code(err)).To(Equal(codes.InvalidArgument))
+		},
+		Entry("unbalanced parens to bypass published", `true) || (true`),
+		Entry("unbalanced closing paren", `true)`),
+		Entry("unbalanced opening paren", `(true`),
+	)
+
+	DescribeTable("validateCELSyntax",
+		func(input string, shouldPass bool) {
+			err := validateCELSyntax(input)
+			if shouldPass {
+				Expect(err).ToNot(HaveOccurred())
+			} else {
+				Expect(err).To(HaveOccurred())
+			}
+		},
+		Entry("valid simple expression", "true", true),
+		Entry("valid field reference", "this.published", true),
+		Entry("valid comparison", "this.id == '123'", true),
+		Entry("valid compound", "this.a && this.b || this.c", true),
+		Entry("unbalanced closing paren", "true)", false),
+		Entry("unbalanced opening paren", "(true", false),
+		Entry("injection attempt", `true) || (true`, false),
+		Entry("empty string is not valid CEL", "", false),
+	)
+})

internal/servers/cluster_catalog_items_server.go

@@ -25,6 +25,7 @@ import (
 	privatev1 "github.com/osac-project/fulfillment-service/internal/api/osac/private/v1"
 	publicv1 "github.com/osac-project/fulfillment-service/internal/api/osac/public/v1"
 	"github.com/osac-project/fulfillment-service/internal/auth"
+	"github.com/osac-project/fulfillment-service/internal/database/dao"
 	"github.com/osac-project/fulfillment-service/internal/events"
 )
 
@@ -41,10 +42,11 @@ var _ publicv1.ClusterCatalogItemsServer = (*ClusterCatalogItemsServer)(nil)
 type ClusterCatalogItemsServer struct {
 	publicv1.UnimplementedClusterCatalogItemsServer
 
-	logger    *slog.Logger
-	delegate  privatev1.ClusterCatalogItemsServer
-	inMapper  *GenericMapper[*publicv1.ClusterCatalogItem, *privatev1.ClusterCatalogItem]
-	outMapper *GenericMapper[*privatev1.ClusterCatalogItem, *publicv1.ClusterCatalogItem]
+	logger           *slog.Logger
+	referenceChecker catalogItemReferenceChecker
+	delegate         privatev1.ClusterCatalogItemsServer
+	inMapper         *GenericMapper[*publicv1.ClusterCatalogItem, *privatev1.ClusterCatalogItem]
+	outMapper        *GenericMapper[*privatev1.ClusterCatalogItem, *publicv1.ClusterCatalogItem]
 }
 
 func NewClusterCatalogItemsServer() *ClusterCatalogItemsServerBuilder {
@@ -101,6 +103,16 @@ func (b *ClusterCatalogItemsServerBuilder) Build() (result *ClusterCatalogItemsS
 		return
 	}
 
+	clustersDao, err := dao.NewGenericDAO[*privatev1.Cluster]().
+		SetLogger(b.logger).
+		SetTenancyLogic(b.tenancyLogic).
+		SetMetricsRegisterer(b.metricsRegisterer).
+		Build()
+	if err != nil {
+		return
+	}
+	referenceChecker := &daoReferenceChecker[*privatev1.Cluster]{resourceDao: clustersDao}
+
 	delegate, err := NewPrivateClusterCatalogItemsServer().
 		SetLogger(b.logger).
 		SetNotifier(b.notifier).
@@ -113,10 +125,11 @@ func (b *ClusterCatalogItemsServerBuilder) Build() (result *ClusterCatalogItemsS
 	}
 
 	result = &ClusterCatalogItemsServer{
-		logger:    b.logger,
-		delegate:  delegate,
-		inMapper:  inMapper,
-		outMapper: outMapper,
+		logger:           b.logger,
+		referenceChecker: referenceChecker,
+		delegate:         delegate,
+		inMapper:         inMapper,
+		outMapper:        outMapper,
 	}
 	return
 }
@@ -126,7 +139,11 @@ func (s *ClusterCatalogItemsServer) List(ctx context.Context,
 	privateRequest := &privatev1.ClusterCatalogItemsListRequest{}
 	privateRequest.SetOffset(request.GetOffset())
 	privateRequest.SetLimit(request.GetLimit())
-	privateRequest.SetFilter(request.GetFilter())
+	composedFilter, err := s.addPublishedFilter(request.GetFilter())
+	if err != nil {
+		return nil, err
+	}
+	privateRequest.SetFilter(composedFilter)
 	privateRequest.SetOrder(request.GetOrder())
 
 	privateResponse, err := s.delegate.List(ctx, privateRequest)
@@ -163,6 +180,16 @@ func (s *ClusterCatalogItemsServer) Get(ctx context.Context,
 		return nil, err
 	}
 
+	if !privateResponse.GetObject().GetPublished() {
+		hasRef, refErr := s.referenceChecker.hasReference(ctx, request.GetId())
+		if refErr != nil {
+			return nil, refErr
+		}
+		if !hasRef {
+			return nil, grpcstatus.Errorf(grpccodes.NotFound, "catalog item not found")
+		}
+	}
+
 	publicCatalogItem := &publicv1.ClusterCatalogItem{}
 	err = s.outMapper.Copy(ctx, privateResponse.GetObject(), publicCatalogItem)
 	if err != nil {
@@ -259,6 +286,16 @@ func (s *ClusterCatalogItemsServer) Update(ctx context.Context,
 	return
 }
 
+func (s *ClusterCatalogItemsServer) addPublishedFilter(filter string) (string, error) {
+	if filter == "" {
+		return "this.published", nil
+	}
+	if err := validateCELSyntax(filter); err != nil {
+		return "", grpcstatus.Errorf(grpccodes.InvalidArgument, "invalid filter: %v", err)
+	}
+	return "(" + filter + ") && this.published", nil
+}
+
 func (s *ClusterCatalogItemsServer) Delete(ctx context.Context,
 	request *publicv1.ClusterCatalogItemsDeleteRequest) (response *publicv1.ClusterCatalogItemsDeleteResponse, err error) {
 	privateRequest := &privatev1.ClusterCatalogItemsDeleteRequest{}