fix(policy): wrong table alias used when injecting for field policies

[ymc9]

fixes #2385

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Resolved an issue where queries with included associations could fail under certain policy configurations.

• Tests

  • Added regression test to ensure policy-enforced data access works correctly when querying related data.

[coderabbitai]

📝 Walkthrough

Walkthrough

A one-line fix in the policy handler changes model-level policy filter construction to use the model name instead of the alias parameter, preventing alias scoping errors in nested relations. A regression test validates the fix handles relation includes with static-value access policies.

Changes

Cohort / File(s)	Summary
Policy Handler Fix packages/plugins/policy/src/policy-handler.ts	Changed buildPolicyFilter invocation to use model name for both parameters instead of alias, fixing SQL scope errors in nested relation queries.
Regression Test tests/regression/test/issue-2385.test.ts	New test validating that User-Association relations with static-value @@allow policies execute without "missing FROM-clause entry" errors.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hoppity-hop, the aliases align,
No more lost scopes in the nested design,
From outer to inner, the model name stays true,
Lateral joins now work right through and through! ✨

🚥 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 directly describes the main change: fixing the use of the wrong table alias when injecting policies for field-level access controls.
Linked Issues check	✅ Passed	The code changes directly address the root cause described in issue #2385 by correcting the alias used in the buildPolicyFilter call, and the regression test validates the fix prevents the FROM-clause error.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the policy filter alias issue: the policy handler fix addresses the bug, and the regression test validates the specific scenario from issue #2385.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/issue-2385

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Claude finished @ymc9's task —— View job

---

Code Review for PR #2413

Reviewing: fix(policy): wrong table alias used when injecting for field policies

Review Tasks

• Analyze the bug fix in policy-handler.ts
• Review the regression test coverage
• Check code quality and best practices
• Assess potential bugs or issues
• Evaluate performance considerations
• Review security concerns

---

Analysis Summary

This PR fixes a critical table alias bug in field policy injection that was causing SQL generation errors when using field-level policies with includes/joins.

Code Quality ✅

Fix Implementation (packages/plugins/policy/src/policy-handler.ts:663):

- const modelPolicyFilter = this.buildPolicyFilter(model, alias, operation);
+ const modelPolicyFilter = this.buildPolicyFilter(model, model, operation);

Strengths:

• Surgical Fix: The change is minimal and precisely targets the root cause
• Clear Intent: The fix correctly uses the model name as the table reference instead of the potentially undefined alias
• Consistent Pattern: Aligns with other similar calls in the same class (e.g., lines 582, 712, 884)

Test Coverage ✅

Regression Test (tests/regression/test/issue-2385.test.ts):

• Good Coverage: Tests the specific scenario that triggered the bug (field-level policies + includes)
• Realistic Schema: Uses a practical User-Association relationship pattern
• Comprehensive: Tests both model-level and field-level policies
• Edge Case: Includes the problematic field-level policy @allow("read", auth().id == userId) on the createdAt field

Potential Issues & Recommendations

Minor Concerns:

1. Test Assertion: The test uses .toResolveTruthy() which only verifies the query doesn't throw an error. Consider adding assertions to verify the actual data returned:

   const result = await userDb.user.findMany({
       include: { associations: true },
   });
   expect(result).toHaveLength(1);
   expect(result[0].associations).toHaveLength(1);

2. Code Documentation: The createSelectAllFieldsWithPolicies method could benefit from JSDoc explaining when alias vs model should be used as the table reference.

Security ✅

• No Security Issues: The fix doesn't introduce any security vulnerabilities
• Policy Integrity: Field-level policies continue to function correctly with proper table references

Performance ✅

• No Performance Impact: The fix only corrects table aliasing; no performance degradation
• SQL Generation: Should actually improve SQL generation correctness, potentially leading to better query optimization

Bug Analysis ✅

Root Cause: When createSelectAllFieldsWithPolicies was called, the alias parameter could be undefined in certain contexts, causing malformed SQL with missing table references in field policy conditions.

Impact: This bug would manifest as "missing FROM-clause entry" database errors when:

• Using field-level policies (like @allow("read", condition))
• Combined with includes/joins
• Where the table alias wasn't properly defined

Fix Quality: The fix correctly identifies that within this method context, the model name itself should be used as the table reference for policy filters, not the potentially undefined alias.

Overall Assessment: ✅ APPROVED

This is a well-executed bug fix with:

• ✅ Minimal, surgical change that addresses the root cause
• ✅ Good regression test coverage
• ✅ No security concerns
• ✅ No performance degradation
• ✅ Follows existing code patterns

The fix correctly resolves the table alias issue in field policy injection and should prevent the "missing FROM-clause entry" errors in complex queries with includes and field-level policies.

---

**

[coderabbitai]

🧹 Nitpick comments (1)

tests/regression/test/issue-2385.test.ts (1)

47-51: Strengthen the assertion to validate returned data, not only non-throw behavior.

This currently proves “no exception,” but it can still pass with an empty result set. Assert row and included relation counts to make the regression stricter.

Suggested test assertion update

-        await expect(
-            userDb.user.findMany({
-                include: { associations: true },
-            }),
-        ).toResolveTruthy();
+        const users = await userDb.user.findMany({
+            include: { associations: true },
+        });
+
+        expect(users).toHaveLength(1);
+        expect(users[0]?.associations).toHaveLength(1);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/regression/test/issue-2385.test.ts` around lines 47 - 51, The test
currently only asserts that userDb.user.findMany with include: { associations:
true } does not throw; instead capture the result into a variable (e.g., const
users = await userDb.user.findMany(...)) and assert the returned data contents:
check users.length > 0 to ensure rows were returned and verify the included
relation(s) are present by asserting each item has an associations array (e.g.,
users.every(u => Array.isArray(u.associations))) and optionally that at least
one user has associations.length > 0 to validate relations were loaded; update
the expectation in the test accordingly (replace the toResolveTruthy() check
with explicit assertions against the users variable).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/regression/test/issue-2385.test.ts`:
- Around line 47-51: The test currently only asserts that userDb.user.findMany
with include: { associations: true } does not throw; instead capture the result
into a variable (e.g., const users = await userDb.user.findMany(...)) and assert
the returned data contents: check users.length > 0 to ensure rows were returned
and verify the included relation(s) are present by asserting each item has an
associations array (e.g., users.every(u => Array.isArray(u.associations))) and
optionally that at least one user has associations.length > 0 to validate
relations were loaded; update the expectation in the test accordingly (replace
the toResolveTruthy() check with explicit assertions against the users
variable).

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 851e744 and e4ab13f.

📒 Files selected for processing (2)

• packages/plugins/policy/src/policy-handler.ts
• tests/regression/test/issue-2385.test.ts