psa-systems/mokosh-server
Watch
9
0
Fork
You've already forked mokosh-server
 1
Code Issues Pull requests Projects Releases 10 Packages Wiki Activity Actions

test(rls): make matching-GUC count consistent with owner count (PMS-430) #317

Closed
David wants to merge 0 commits from fix/PMS-430-rls-isolation-guc-count-internal-consistency into main
pull from: fix/PMS-430-rls-isolation-guc-count-internal-consistency
merge into: psa-systems:main
Conversation 0 Commits - Files changed 447 +10 -6
David commented 2026-06-19 12:30:37 +02:00
Owner
Copy link

The rls_isolation regression broke on main from a semantic merge conflict between two independently-correct branch edits to the same assertion. PMS-414 (#310) changed the assertion to compare the matching-GUC read against tenant_a_companies, the owner-visible count(*) WHERE tenant_id = $1, which includes the internal own-company that PMS-413 migration 062 backfills for the default tenant (= 2). PMS-413 separately added WHERE company_type <> 'internal' to the GUC count (= 1), which was consistent only against the old hard-coded 1. Merged together the two counts use different filters, so the assertion compares 1 against 2 and fails ("tenant-A GUC returned 2 rows instead of the expected 1").

The internal own-company is a legitimate tenant-A row that RLS must expose under the matching GUC, so the GUC count must include it to match the owner count. Drop the <> 'internal' filter from the GUC count so both sides count the same set; the test still proves fail-closed reads (zero rows with no GUC) and that the matching GUC exposes exactly tenant A's rows.

#PMS-430

The rls_isolation regression broke on main from a semantic merge conflict between two independently-correct branch edits to the same assertion. PMS-414 (#310) changed the assertion to compare the matching-GUC read against tenant_a_companies, the owner-visible count(*) WHERE tenant_id = $1, which includes the internal own-company that PMS-413 migration 062 backfills for the default tenant (= 2). PMS-413 separately added WHERE company_type <> 'internal' to the GUC count (= 1), which was consistent only against the old hard-coded 1. Merged together the two counts use different filters, so the assertion compares 1 against 2 and fails ("tenant-A GUC returned 2 rows instead of the expected 1"). The internal own-company is a legitimate tenant-A row that RLS must expose under the matching GUC, so the GUC count must include it to match the owner count. Drop the <> 'internal' filter from the GUC count so both sides count the same set; the test still proves fail-closed reads (zero rows with no GUC) and that the matching GUC exposes exactly tenant A's rows. #PMS-430
test(rls): make matching-GUC count consistent with owner count (PMS-430)
Some checks failed
E2E / Playwright against staging (pull_request) Failing after 37s
Details
Check / fmt + clippy + compile + unit/doc tests (pull_request) Successful in 1m7s
Details
Integration / integration tests (pull_request) Successful in 4m16s
Details
Create release / Create release from merged PR (pull_request) Has been skipped
Details
02cdce95f9 
The rls_isolation regression broke on main from a semantic merge conflict between two independently-correct branch edits to the same assertion. PMS-414 (#310) changed the assertion to compare the matching-GUC read against tenant_a_companies, the owner-visible count(*) WHERE tenant_id = $1, which includes the internal own-company that PMS-413 migration 062 backfills for the default tenant (= 2). PMS-413 separately added WHERE company_type <> 'internal' to the GUC count (= 1), which was consistent only against the old hard-coded 1. Merged together the two counts use different filters, so the assertion compares 1 against 2 and fails ("tenant-A GUC returned 2 rows instead of the expected 1").

The internal own-company is a legitimate tenant-A row that RLS must expose under the matching GUC, so the GUC count must include it to match the owner count. Drop the <> 'internal' filter from the GUC count so both sides count the same set; the test still proves fail-closed reads (zero rows with no GUC) and that the matching GUC exposes exactly tenant A's rows.

#PMS-430
nrupard closed this pull request 2026-06-20 19:03:13 +02:00
nrupard deleted branch fix/PMS-430-rls-isolation-guc-count-internal-consistency 2026-06-20 19:03:17 +02:00
Some checks failed
E2E / Playwright against staging (pull_request) Failing after 37s
Required
Details
Check / fmt + clippy + compile + unit/doc tests (pull_request) Successful in 1m7s
Required
Details
Integration / integration tests (pull_request) Successful in 4m16s
Details
Create release / Create release from merged PR (pull_request) Has been skipped
Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!317
No description provided.