[AI] OSAC-4127: List DAO tenancy filter bypassed by top-level OR in user filter (cross-tenant resolution by name)

[osac-jira-ai-issue-solver]

Resolves OSAC-4127

Summary

• Fix SQL operator precedence bug in the generic List DAO that allowed tenant isolation to be bypassed when a user-supplied CEL filter contained a top-level OR (e.g., this.id == k || this.metadata.name == k)
• The user filter is now wrapped in parentheses before being appended to the tenancy clause, ensuring the tenant restriction applies to the entire filter expression
• Add DAO-level regression test that verifies cross-tenant rows are not leaked through top-level OR filters

Test plan

• go build ./internal/database/dao/... compiles successfully
• go vet ./internal/database/dao/... passes
• golangci-lint run ./internal/database/dao/... reports 0 issues
• gofmt -s produces no changes
• ginkgo run -r internal — requires PostgreSQL container (CI will run this)
• Verify the new test case "Does not leak cross-tenant rows when filter has a top-level OR" passes in CI

Fixes: OSAC-4127

[osac-jira-ai-issue-solver]

AI Session Costs

Session	Cost
New ticket	$2.09
Feedback (1) (no changes)	$0.54
Feedback (1) retry 1 (no changes)	$0.46
Feedback (1) retry 2 (no changes)	$0.29
Feedback (1) retry 3	$0.54
Feedback (2) (no changes)	$0.31
Feedback (2) retry 1 (no changes)	$0.34
Feedback (2) retry 2 (no changes)	$0.41
Feedback (2) retry 3 (unable)	$0.56
Feedback (3) (no changes)	$0.85
Feedback (3) retry 1 (no changes)	$0.64
Feedback (3) retry 2 (no changes)	$0.59
Feedback (3) retry 3 (unable)	$0.63
Total	$8.25

[openshift-ci-robot]

@osac-jira-ai-issue-solver[bot]: This pull request references OSAC-4127 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Resolves OSAC-4127

Summary

• Fix SQL operator precedence bug in the generic List DAO that allowed tenant isolation to be bypassed when a user-supplied CEL filter contained a top-level OR (e.g., this.id == k || this.metadata.name == k)
• The user filter is now wrapped in parentheses before being appended to the tenancy clause, ensuring the tenant restriction applies to the entire filter expression
• Add DAO-level regression test that verifies cross-tenant rows are not leaked through top-level OR filters

Test plan

• go build ./internal/database/dao/... compiles successfully
• go vet ./internal/database/dao/... passes
• golangci-lint run ./internal/database/dao/... reports 0 issues
• gofmt -s produces no changes
• ginkgo run -r internal — requires PostgreSQL container (CI will run this)
• Verify the new test case "Does not leak cross-tenant rows when filter has a top-level OR" passes in CI

Fixes: OSAC-4127

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:42 PM UTC · Completed 4:55 PM UTC

Commit: eecbc32 · View workflow run →

[fullsend-ai-review]

Looks good to me

Previous run

Looks good to me

Previous run (2)

Review

Findings

Low

• [naming-convention] fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go:353 — Test description "Does not leak cross-tenant rows when filter has a top-level OR" uses a negation form not seen in other test descriptions in this file. Other tests use positive third-person verb patterns: "Filters field based on...", "Shows all tenants when...", "Allows a tenant to...", "Rejects update of...". Consider rephrasing to follow the established pattern, e.g., "Isolates tenant rows when filter has a top-level OR".

---

Labels: PR fixes a tenant isolation bypass (security bug) in the fulfillment-service DAO layer

[osac-jira-ai-issue-solver]

In comment, @fullsend-ai-review[bot] said:

🤖 Finished Review · ✅ Success · Started 4:42 PM UTC · Completed 4:55 PM UTC

Commit: eecbc32 · View workflow run →

Automated bot status notification — no action needed.

Addressed in 63e1906.

[osac-jira-ai-issue-solver]

In comment, @fullsend-ai-review[bot] said:

Review

Findings

Low

• [naming-convention] fulfillment-service/internal/database/dao/generic_dao_tenant_visibility_test.go:353 — Test description "Does not leak cross-tenant rows when filter has a top-level OR" uses a negation form not seen in other test descriptions in this file. Other tests use positive third-person verb patterns: "Filters field based on...", "Shows all tenants when...", "Allows a tenant to...", "Rejects update of...". Consider rephrasing to follow the established pattern, e.g., "Isolates tenant rows when filter has a top-level OR".

---

Labels: PR fixes a tenant isolation bypass (security bug) in the fulfillment-service DAO layer

Review summary comment — findings addressed via the line-level comment above.

Addressed in 63e1906.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:02 PM UTC · Completed 5:15 PM UTC

Commit: 63e1906 · View workflow run →

[osac-jira-ai-issue-solver]

In comment, @fullsend-ai-review[bot] said:

🤖 Finished Review · ✅ Success · Started 5:02 PM UTC · Completed 5:15 PM UTC

Commit: 63e1906 · View workflow run →

I was unable to produce code changes to address this comment after multiple attempts.

[slintes]

@osac-jira-ai-issue-solver monitor this PR and rebase on main when it's merged, that will fix CI failures: #367

[osac-jira-ai-issue-solver]

In comment, @slintes said:

@osac-jira-ai-issue-solver monitor this PR and rebase on main when it's merged, that will fix CI failures: #367

I was unable to produce code changes to address this comment after multiple attempts.

[slintes]

@osac-jira-ai-issue-solver rebase on main

[slintes]

/fs-fix rebase on main to fix unit tests

[slintes]

rebased myself 🤷🏼‍♂️

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:42 PM UTC · Completed 6:56 PM UTC

Commit: baf1061 · View workflow run →

[omer-vishlitzky]

/lgtm

[omer-vishlitzky]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: omer-vishlitzky, osac-jira-ai-issue-solver[bot], slintes

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• fulfillment-service/OWNERS [omer-vishlitzky]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment