fix(orm): forbid selecting @omit fields when allowQueryTimeOmitOverride is false

[ymc9]

Problem

Fixes #2671.

Fields marked with @omit are excluded from default query results, but they could be leaked by explicitly selecting them via select: { field: true }. Setting allowQueryTimeOmitOverride: false was supposed to forbid un-omitting fields at query time, but it only guarded the omit clause — the select-based override path remained open.

// allowQueryTimeOmitOverride: false
await db.user.findFirst({ select: { passwordHash: true } });
// previously: { passwordHash: "$2a$12$..." }  ← leak

Fix

makeSelectSchema now consults a new isFieldOmittedByConfig helper (checking options-level and schema-level omit; query-level is excluded as it's mutually exclusive with select). When allowQueryTimeOmitOverride === false and a field is omitted by config, its select schema only accepts false (z.literal(false)), so select: { field: true } is rejected by validation — consistent with how omit: { field: false } is already rejected. This applies recursively to nested relation selects.

Default behavior (allowQueryTimeOmitOverride unset/true) is unchanged: explicit select of an @omit field still returns it, by design.

Tests

Added tests/regression/test/issue-2671.test.ts:

• default: explicit select of an @omit field returns it
• allowQueryTimeOmitOverride: false: select: { passwordHash: true } is rejected; selecting non-omitted fields and select: { passwordHash: false } still work

Existing omit.test.ts suite still passes.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Fixed field selection behavior for omitted fields when query-time omit override is disabled. Omitted fields now correctly reject selection attempts and enforce validation rules, while non-omitted fields remain accessible.

• Tests

  • Added regression test suite to verify field omission handling under different query-time override configurations.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR fixes a security vulnerability where fields marked @omit could be bypassed using explicit select statements. It introduces configuration-aware field omit checking and conditionally constrains generated schemas to enforce omit restrictions when query-time override is disabled.

Changes

Omit Field Protection

Layer / File(s)	Summary
Field omit configuration helper packages/orm/src/client/zod/factory.ts	New isFieldOmittedByConfig(model, field) checks whether a field is omitted in options.omit (supporting both lowercased and direct model keys) and falls back to schema-level fieldDef.omit.
Select schema omit enforcement packages/orm/src/client/zod/factory.ts	Modified makeSelectSchema to restrict scalar select fields when allowQueryTimeOmitOverride === false: omitted fields only permit z.literal(false).optional() instead of any boolean.
Regression test coverage tests/regression/test/issue-2671.test.ts	Added test suite validating behavior with @omit field: default allows query-time override (field selection works), and disabled override rejects field selection while allowing field exclusion.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A password once hidden, now guarded with care,
With @omit enforced beyond what was there,
No sneaky select can bypass the wall,
The schema now knows, and protects it all!

🚥 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 accurately summarizes the main fix: forbidding selection of @omit fields when allowQueryTimeOmitOverride is false, which directly addresses the core issue being resolved.
Linked Issues check	✅ Passed	The PR fully addresses issue #2671 by implementing validation that prevents @omit fields from being returned when allowQueryTimeOmitOverride is false, meeting the expected behavior requirement.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing issue #2671: the factory.ts modification implements the core fix via isFieldOmittedByConfig helper, and the test file adds comprehensive regression coverage.
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

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-2671-omit-select-leak

---

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

---

PR Review: Security Fix for @omit Field Selection Leak

Review Tasks:

• Analyze the core implementation changes in factory.ts
• Review the new regression test coverage
• Assess security implications and completeness of the fix
• Check for potential edge cases or bugs
• Evaluate code quality and maintainability

Summary

This PR effectively addresses the security vulnerability where @omit fields could be leaked through explicit select clauses when allowQueryTimeOmitOverride: false. The implementation is clean, well-tested, and follows existing patterns in the codebase.

Detailed Review

✅ Security Fix Quality

The core fix in makeSelectSchema() (factory.ts:1156-1162) is excellent:

• Precise targeting: Only affects the specific vulnerability path without changing default behavior
• Consistent validation: Uses z.literal(false) matching the existing omit clause validation approach
• Recursive protection: Applied through z.lazy(() => this.makeSelectSchema(...)) for nested relations (factory.ts:1236)
• Clear intent: Well-commented security rationale

✅ Implementation Quality

Helper Function (factory.ts:1274-1286):

• Good separation of concerns: Extracted isFieldOmittedByConfig() keeps logic modular
• Comprehensive coverage: Handles both options-level and schema-level omit configurations
• Proper type checking: Uses typeof omitConfig[field] === 'boolean' for safety
• Consistent with existing patterns: Follows same model/field lookup patterns as other helpers

Integration:

• Applied consistently across all CRUD operations that support select
• Maintains backward compatibility (default behavior unchanged)
• Works recursively for nested relation selects

✅ Test Coverage

Regression Test (issue-2671.test.ts) provides comprehensive coverage:

• Default behavior verification: Confirms existing functionality still works
• Security enforcement: Validates rejection of select: { passwordHash: true }
• Normal operations: Ensures non-omitted field selection still works
• Edge case: Tests explicit exclusion select: { passwordHash: false } is allowed
• Uses realistic scenario: Email/passwordHash example matches real-world usage

Existing tests compatibility: The fix aligns perfectly with existing omit.test.ts which already tests allowQueryTimeOmitOverride: false for the omit clause (omit.test.ts:144-161).

✅ Performance & Security

• No performance impact: Only adds a lightweight check during schema generation (cached)
• Complete security coverage: No bypasses identified - both direct select and nested relation selects are protected
• Fail-safe approach: Validation happens at schema level, preventing malformed queries from executing

Minor Observations

1. Documentation consistency: The JSDoc for isFieldOmittedByConfig is excellent and clearly explains the exclusion of query-level omit.

2. Type safety: The z.literal(false) approach provides compile-time and runtime safety.

3. Edge case handling: Properly handles both lowerCaseFirst(model) and model casing for options-level omit.

Conclusion

This is a high-quality security fix that:

• ✅ Completely addresses the vulnerability without breaking changes
• ✅ Follows existing architectural patterns and validation strategies
• ✅ Includes thorough test coverage for all scenarios
• ✅ Maintains backward compatibility while enforcing security when configured
• ✅ Has no performance implications or side effects

Recommendation: ✅ Approve - This PR effectively closes a security gap with minimal, focused changes and comprehensive testing.

[coderabbitai]

🧹 Nitpick comments (1)

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

29-58: ⚡ Quick win

Add one nested-relation regression to lock the recursive enforcement contract.

This suite verifies top-level select enforcement well, but it doesn’t assert the same rule through nested relation select paths. Adding one relation case would harden the security regression surface against future recursion regressions.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/regression/test/issue-2671.test.ts` around lines 29 - 58, The test
misses asserting nested-relation enforcement of omitted fields; add a case that
creates a related record (e.g., create a Profile or related model tied to user)
and call db.user.findFirst with a nested relation select that attempts to
include the omitted field (use db.user.findFirst with select: { relationName: {
select: { passwordHash: true } } }) and assert it is rejected via
toBeRejectedByValidation when allowQueryTimeOmitOverride is false; also add a
complementary nested select that only picks non-omitted fields to assert it
still succeeds (use select: { relationName: { select: { email: true } } } and
expect the returned shape).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/regression/test/issue-2671.test.ts`:
- Around line 29-58: The test misses asserting nested-relation enforcement of
omitted fields; add a case that creates a related record (e.g., create a Profile
or related model tied to user) and call db.user.findFirst with a nested relation
select that attempts to include the omitted field (use db.user.findFirst with
select: { relationName: { select: { passwordHash: true } } }) and assert it is
rejected via toBeRejectedByValidation when allowQueryTimeOmitOverride is false;
also add a complementary nested select that only picks non-omitted fields to
assert it still succeeds (use select: { relationName: { select: { email: true }
} } and expect the returned shape).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5542b3f3-52e4-4142-a795-ae17f27b6349

📥 Commits

Reviewing files that changed from the base of the PR and between 0da9f81 and 99745a4.

📒 Files selected for processing (2)

• packages/orm/src/client/zod/factory.ts
• tests/regression/test/issue-2671.test.ts