Skip to content

fix(policy): compare before() enum fields as text on PostgreSQL - #2869

Open
theluckystrike wants to merge 1 commit into
zenstackhq:devfrom
theluckystrike:fix/post-update-before-enum-pg
Open

theluckystrike wants to merge 1 commit into
zenstackhq:devfrom
theluckystrike:fix/post-update-before-enum-pg

Conversation

@theluckystrike

@theluckystrike theluckystrike commented Oct 5, 2026 •

Copy link
Copy Markdown

Fixes #2727

What broke. On PostgreSQL with a native enum column, a post-update rule that compares before().field with the current field fails with operator does not exist: text <> "State" instead of evaluating the rule.

Root cause. The $before table for post-update checks is built by buildValuesTableSelect, and the PostgreSQL dialect casts every enum field there to text (packages/orm/src/client/crud/dialects/postgresql.ts:503-505, used at :657). The current row's column keeps its native enum type, and _binary in packages/plugins/policy/src/expression-transformer.ts:325-337 compares the two as they are. PostgreSQL has no operator between text and a user enum type.

Fix. In _binary, when either operand is a before() access to an enum field, both operands go through dialect.castText before buildComparison. The in branch already does the same for enum fields. Comparisons that don't involve before() enums are unchanged, so normal enum filters keep using the column as is.

How I tested it.

  • New regression test tests/regression/test/issue-2727.test.ts runs on PostgreSQL with usePrismaPush: true.
  • On dev it fails with operator does not exist: text <> "State". With the fix the denied transition is rejected by policy and the allowed one goes through.
  • I also tried the same rule with @map on the enum values and @@map on the enum. It passes with the fix (not added as a test).
  • tests/e2e orm/policy on PostgreSQL 16 and SQLite gives the same results with and without the change. A few tests fail locally on both (for example the now() tests on PostgreSQL), so that's my environment.
  • pnpm lint in packages/plugins/policy and tsc --noEmit in tests/regression are clean.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed policy checks for updates involving enum fields accessed through before(), so comparisons correctly distinguish allowed and rejected state changes.

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>
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
CLAUDE.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: zenstackhq/zenstack/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4539d31f-0923-441f-8f27-547ab88fe06c
📥 Commits

Reviewing files that changed from the base of the PR and between e7ba2fa and ae35b2b.

📒 Files selected for processing (2)
  • packages/plugins/policy/src/expression-transformer.ts
  • tests/regression/test/issue-2727.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The policy expression transformer now detects comparisons involving a before() enum field and casts both operands to text. A PostgreSQL regression test checks rejected and accepted state transitions.

Changes

PostgreSQL enum policy comparisons

Layer / File(s) Summary
Transform and validate enum comparisons
packages/plugins/policy/src/expression-transformer.ts, tests/regression/test/issue-2727.test.ts
The transformer detects a before() enum field and casts both comparison operands to text. The regression test checks that a transition to DONE is rejected and a transition to IN_PROGRESS succeeds.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: ymc9

Merge Risk: ⚪ Minimal · up to ae35b

No actionable merge-blocking risk was established for the PostgreSQL enum comparison fix; it is ready for normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ae35b

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

  • Medium · security · inferred: Atomic denial is unresolved when a caller catches rejection inside an existing transaction. The mutation executes before the post-update check, and the inspected executor rolls back only transactions it starts. This PR changes affected native-enum comparisons from PostgreSQL type errors to application-level policy rejection. A caught rejection could therefore leave the denied update committable unless the surrounding transaction supplies an additional abort or rollback guarantee. The rollback dependency predates this PR; the concern is its exposure through the newly executable comparison.
Security review details

Security Blast Radius

  • inferred — The identified exposure is bounded to updates governed by affected before-enum post-update predicates, particularly PostgreSQL native enums. Exploitation of the unresolved recovery path would additionally require an application integration that continues an existing transaction after catching rejection. No direct tenant or infrastructure privilege expansion is established.

Security Findings and Attack Paths

  • inferred — A conditional attack path is an otherwise permitted update containing a policy-forbidden enum transition, followed by application handling that catches the post-update rejection and commits the existing transaction. The update precedes rejection, but whether the surrounding transaction prevents this outcome remains unverified. Automatically managed rollback blocks this path.

Trust Boundaries and Controls

  • observed — The changed code constructs SQL predicate nodes through existing dialect operations; it does not introduce a runtime policy parser or change the before-row join and policy-count controls.

Hardening Proposals

  • proposed — Establish the recovery contract for policy rejection caught inside an existing transaction: either undo the rejected mutation or prevent that transaction from committing. Validate the contract by checking persisted state after rejection and attempted continuation.
🚥 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 describes the main change: comparing before() enum fields as text on PostgreSQL.
Linked Issues check ✅ Passed Issue #2727 requires PostgreSQL post-update policy comparisons between a native enum's before() value and its current value to avoid the text-to-enum operator error. _binary detects a before() e…
Out of Scope Changes check ✅ Passed The reported changes are limited to the policy comparison transformation and a regression test for issue #2727. Both changes directly support the requested native-enum post-update comparison behavior.…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/plugins/policy/src/expression-transformer.ts

ESLint 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.

❤️ Share

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

This branch has not been deployed

No deployments
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.

before() function in post-update policy rules fails with PostgreSQL native enums — operator does not exist: text <> "EnumType"

1 participant