Repository navigation
fix(policy): compare before() enum fields as text on PostgreSQL - #2869
theluckystrike wants to merge 1 commit into
Conversation
The post-update `$before` VALUES table casts enum fields to text, so a rule like `before().state != state` produced `text <> "State"`, which PostgreSQL rejects for native enum columns. Cast both operands to text when one side is a `before()` enum field. Fixes zenstackhq#2727 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe policy expression transformer now detects comparisons involving a ChangesPostgreSQL enum policy comparisons
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk was established for the PostgreSQL enum comparison fix; it is ready for normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The fix preserves policy enforcement and rollback for automatically managed transactions. Rollback remains unresolved when an application catches a rejected update inside an already-open transaction. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
packages/plugins/policy/src/expression-transformer.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. 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. Comment |
Fixes #2727
What broke. On PostgreSQL with a native enum column, a post-update rule that compares
before().fieldwith the current field fails withoperator does not exist: text <> "State"instead of evaluating the rule.Root cause. The
$beforetable for post-update checks is built bybuildValuesTableSelect, and the PostgreSQL dialect casts every enum field there totext(packages/orm/src/client/crud/dialects/postgresql.ts:503-505, used at:657). The current row's column keeps its native enum type, and_binaryinpackages/plugins/policy/src/expression-transformer.ts:325-337compares the two as they are. PostgreSQL has no operator betweentextand a user enum type.Fix. In
_binary, when either operand is abefore()access to an enum field, both operands go throughdialect.castTextbeforebuildComparison. Theinbranch already does the same for enum fields. Comparisons that don't involvebefore()enums are unchanged, so normal enum filters keep using the column as is.How I tested it.
tests/regression/test/issue-2727.test.tsruns on PostgreSQL withusePrismaPush: true.devit fails withoperator does not exist: text <> "State". With the fix the denied transition is rejected by policy and the allowed one goes through.@mapon the enum values and@@mapon the enum. It passes with the fix (not added as a test).tests/e2eorm/policyon PostgreSQL 16 and SQLite gives the same results with and without the change. A few tests fail locally on both (for example thenow()tests on PostgreSQL), so that's my environment.pnpm lintinpackages/plugins/policyandtsc --noEmitintests/regressionare clean.Summary by CodeRabbit
before(), so comparisons correctly distinguish allowed and rejected state changes.