From 6e5cd6682f18590938f6aa43e48e493604828b89 Mon Sep 17 00:00:00 2001 From: "osac-jira-ai-issue-solver[bot]" <280426608+osac-jira-ai-issue-solver[bot]@users.noreply.github.com> Date: Tue, 18 Aug 2026 16:41:03 +0000 Subject: [PATCH 1/2] OSAC-4127: List DAO tenancy filter bypassed by top-level OR in user filter (cross-tenant resolution by name) Co-authored-by: Marc Sluiter --- .../internal/database/dao/generic_dao_list.go | 2 + .../dao/generic_dao_tenant_visibility_test.go | 56 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/fulfillment-service/internal/database/dao/generic_dao_list.go b/fulfillment-service/internal/database/dao/generic_dao_list.go index ae15406eef..7c479d1f6c 100644 --- a/fulfillment-service/internal/database/dao/generic_dao_list.go +++ b/fulfillment-service/internal/database/dao/generic_dao_list.go @@ -91,7 +91,9 @@ func (r *ListRequest[O]) do(ctx context.Context) (response *ListResponse[O], err if r.sql.filter.Len() > 0 { r.sql.filter.WriteString(` and `) } + r.sql.filter.WriteString(`(`) r.sql.filter.WriteString(filter) + r.sql.filter.WriteString(`)`) } // Calculate the order clause: diff --git a/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go b/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go index 0d63274d90..68a1d48c03 100644 --- a/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go +++ b/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go @@ -351,6 +351,62 @@ var _ = Describe("Tenant visibility", func() { Expect(getResponse.GetObject().GetMyString()).To(Equal("updated")) }) + It("Does not leak cross-tenant rows when filter has a top-level OR", func() { + // Create an unrestricted tenancy to seed objects in different tenants: + tenancyAll := auth.NewMockTenancyLogic(ctrl) + tenancyAll.EXPECT().DetermineVisibleTenants(gomock.Any()). + Return(collections.NewUniversalSet[string](), nil). + AnyTimes() + daoAll, err := NewGenericDAO[*testsv1.Object](). + SetLogger(logger). + SetTenancyLogic(tenancyAll). + Build() + Expect(err).ToNot(HaveOccurred()) + + // Create an object in tenant-a (will be visible): + _, err = daoAll.Create(). + SetObject(testsv1.Object_builder{ + Metadata: testsv1.Metadata_builder{ + Tenant: "tenant-a", + Name: "shared-name", + }.Build(), + }.Build()). + Do(ctx) + Expect(err).ToNot(HaveOccurred()) + + // Create an object in tenant-b with the same name (should be invisible to tenant-a): + _, err = daoAll.Create(). + SetObject(testsv1.Object_builder{ + Metadata: testsv1.Metadata_builder{ + Tenant: "tenant-b", + Name: "shared-name", + }.Build(), + }.Build()). + Do(ctx) + Expect(err).ToNot(HaveOccurred()) + + // Create a restricted DAO that can only see tenant-a: + tenancyRestricted := auth.NewMockTenancyLogic(ctrl) + tenancyRestricted.EXPECT().DetermineVisibleTenants(gomock.Any()). + Return(collections.NewSet("tenant-a"), nil). + AnyTimes() + daoRestricted, err := NewGenericDAO[*testsv1.Object](). + SetLogger(logger). + SetTenancyLogic(tenancyRestricted). + Build() + Expect(err).ToNot(HaveOccurred()) + + // List with a top-level OR filter (id-or-name pattern). The tenancy clause + // must apply to the entire filter, not just the first branch: + listResponse, err := daoRestricted.List(). + SetFilter(`this.id == "nonexistent" || this.metadata.name == "shared-name"`). + Do(ctx) + Expect(err).ToNot(HaveOccurred()) + Expect(listResponse.GetTotal()).To(Equal(int32(1))) + Expect(listResponse.GetItems()).To(HaveLen(1)) + Expect(listResponse.GetItems()[0].GetMetadata().GetTenant()).To(Equal("tenant-a")) + }) + It("Rejects update of an object belonging to an invisible tenant as not found", func() { // Create a tenancy logic that makes all tenants visible, used to create the object: tenancyA := auth.NewMockTenancyLogic(ctrl) From baf1061dafc3c85f545d3439ddd442ec6c699abd Mon Sep 17 00:00:00 2001 From: "osac-jira-ai-issue-solver[bot]" <280426608+osac-jira-ai-issue-solver[bot]@users.noreply.github.com> Date: Tue, 18 Aug 2026 17:01:08 +0000 Subject: [PATCH 2/2] OSAC-4127: address PR feedback Co-authored-by: Marc Sluiter --- .../internal/database/dao/generic_dao_tenant_visibility_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go b/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go index 68a1d48c03..8f8509d81b 100644 --- a/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go +++ b/fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go @@ -351,7 +351,7 @@ var _ = Describe("Tenant visibility", func() { Expect(getResponse.GetObject().GetMyString()).To(Equal("updated")) }) - It("Does not leak cross-tenant rows when filter has a top-level OR", func() { + It("Isolates tenant rows when filter has a top-level OR", func() { // Create an unrestricted tenancy to seed objects in different tenants: tenancyAll := auth.NewMockTenancyLogic(ctrl) tenancyAll.EXPECT().DetermineVisibleTenants(gomock.Any()).