Skip to content

test(regression): add regression test for issue #2538#2607

Merged
ymc9 merged 1 commit intodevfrom
fix/regression-test-2538
Apr 22, 2026
Merged

test(regression): add regression test for issue #2538#2607
ymc9 merged 1 commit intodevfrom
fix/regression-test-2538

Conversation

@ymc9
Copy link
Copy Markdown
Member

@ymc9 ymc9 commented Apr 22, 2026

Summary

  • Adds a regression test for issue #2538: nested collection predicates in access policies generating invalid PostgreSQL SQL
  • The test covers a schema with deeply nested ?[] predicates: project.userPermissions?[user == auth() && accessType == 'SPECIFIC_ROLE' && role.rolePermissions?[area == 'TestCaseRepository' && canAddEdit]]
  • Verifies that admin access, authorized user creation, and unauthorized user rejection all behave correctly

Test plan

  • TEST_PG_USER=<user> pnpm exec vitest run test/issue-2538.test.ts passes in tests/regression/

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added regression test to validate correct execution of access control policies with nested collection predicates in database queries.

Adds a regression test for nested collection predicates in access
policies generating valid SQL (issue #2538).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7bb268c7-76c0-4900-9c45-471cf114f162

📥 Commits

Reviewing files that changed from the base of the PR and between 40c4594 and fa94d38.

📒 Files selected for processing (1)
  • tests/regression/test/issue-2538.test.ts

📝 Walkthrough

Walkthrough

A new regression test is added for issue-2538 that validates the correct compilation and execution of authorization rules containing nested collection predicates. The test creates a policy-aware database with multiple user roles and permission levels, then asserts policy enforcement across different access scenarios.

Changes

Cohort / File(s) Summary
Regression Test
tests/regression/test/issue-2538.test.ts
Regression test for nested collection predicates within @@allow('create', ...) rules, validating policy execution across admin, authorized, and unauthorized users with role-based permissions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Nested predicates, now they're right!
Testing permissions day and night!
Admin, auth'd, denied with care,
Policy rules work everywhere! 🔒
A regression test hops into place,
Protecting authorization's space! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically identifies the main change: adding a regression test for issue #2538, matching the file added and PR objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/regression-test-2538

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 22, 2026

Claude finished @ymc9's task —— View job


Code Review for Regression Test #2538

Reviewing the regression test for nested collection predicates in access policies.

Todo List

  • Read and analyze the new test file
  • Review code quality and structure
  • Check test coverage and effectiveness
  • Evaluate for potential issues
  • Provide comprehensive feedback

Review Feedback

Code Quality & Structure

Strengths:

  • Follows established ZenStack regression test patterns consistently
  • Clear, descriptive test name that matches the GitHub issue title
  • Proper use of createPolicyTestClient for policy testing
  • Well-structured schema with realistic domain models (User, Project, Role, etc.)
  • Good use of descriptive comments explaining test scenarios (tests/regression/test/issue-2538.test.ts:99, tests/regression/test/issue-2538.test.ts:105, tests/regression/test/issue-2538.test.ts:110)

Code Organization:

  • Schema definition is inline and comprehensive
  • Test setup follows a logical flow: create entities → set up relationships → test scenarios
  • Appropriate use of ZenStack's custom Vitest matchers (toResolveTruthy, toBeRejectedByPolicy)

Test Coverage & Effectiveness

Comprehensive Test Scenarios:
The test effectively covers the core issue with three critical scenarios:

  1. Admin bypass (tests/regression/test/issue-2538.test.ts:100): Verifies that SQL generation works even when admin policies coexist with complex nested predicates
  2. Authorized access (tests/regression/test/issue-2538.test.ts:106-108): Tests the exact nested predicate scenario from the issue
  3. Unauthorized rejection (tests/regression/test/issue-2538.test.ts:111-113): Ensures proper access control enforcement

Complex Predicate Testing:
The nested predicate is appropriately complex and mirrors real-world scenarios:

project.userPermissions?[user == auth() && accessType == 'SPECIFIC_ROLE' &&
    role.rolePermissions?[area == 'TestCaseRepository' && canAddEdit]]

Schema Design

Realistic Domain Model:

  • Well-designed permission system with Users, Projects, Roles, and Permissions
  • Proper foreign key relationships and constraints
  • Appropriate use of @@allow policies for testing different access patterns

⚠️ Minor Suggestions

  1. Test Data Constants: Consider extracting magic strings to constants for better maintainability:

    const TEST_AREA = 'TestCaseRepository';
    const ADMIN_ACCESS = 'ADMIN';
    const USER_ACCESS = 'USER';
    const SPECIFIC_ROLE_TYPE = 'SPECIFIC_ROLE';
  2. Error Message Validation: The toBeRejectedByPolicy() calls could optionally include expected error messages to make failures more diagnostic.

  3. Test Coverage Gap: Consider adding a test case for a user with no permissions at all to ensure complete coverage.

Performance & Security

No Concerns:

  • Test uses appropriate database setup via createPolicyTestClient
  • No performance anti-patterns detected
  • Schema follows security best practices with explicit policy declarations
  • No sensitive data or credentials present

Integration with Existing Codebase

Excellent Integration:

  • Follows exact naming convention: issue-{number}.test.ts
  • Uses correct imports and test utilities
  • Matches style and structure of other regression tests
  • Includes proper issue reference comment

Summary

This is a well-implemented regression test that effectively validates the fix for nested collection predicates in PostgreSQL SQL generation. The test is comprehensive, follows established patterns, and should provide reliable protection against regression of this specific issue.

Recommendation: ✅ Approve - The test is ready for merge with no blocking issues.


Job Run: View Details

@ymc9 ymc9 merged commit 09ca875 into dev Apr 22, 2026
8 checks passed
@ymc9 ymc9 deleted the fix/regression-test-2538 branch April 22, 2026 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant