fix!: harden filter validation and adapter state preservation - #766
Conversation
Validation: fully-rejected compounds prune away (schema defaults apply again for nested input), elemMatch interiors run through the validate hook inside-out, and validation short-circuits when no hook is configured. The Validator type no longer admits a void return, so inspect-only hooks must return the filter instead of silently rejecting every leaf. Parsers: dotted keys and nested objects sharing a prefix merge instead of order-dependently replacing each other; the traversal depth cap and the defaults fallback are owned by @rapiq/core and shared by the resolver and the expression/mongo dialects; sync/async parse pipelines share one front-end per parser. TypeORM: filter parameters bind under a per-run namespace so caller-owned bindings (or a previous run) are never rebound; queries without sorts or pagination leave caller-owned ORDER BY/take/skip untouched, mirroring the WHERE preservation contract. SQL: regex values that are neither RegExp nor string throw a typed AdapterError instead of binding raw; the default relation alias is bounded to 63 characters with a hash suffix so database identifier truncation cannot collapse long distinct paths. CI: coverage thresholds are enforced in branch/PR CI instead of first failing between release-please tagging and npm publish. BREAKING CHANGE: the filters Validator type returns MaybeAsync<IFilter | undefined> (no void); relation aliases longer than 63 characters gain a hash suffix; TypeORM filter parameters are named rapiq_<n>_<i> instead of positional 0, 1, ...
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR hardens filter validation and defaults across dialects, centralizes traversal limits, merges dotted filter keys, validates SQL regex inputs, bounds relation aliases, preserves TypeORM builder state and parameter bindings, updates migration documentation, and runs CI tests with coverage. ChangesFilter parsing and validation hardening
SQL expression and relation alias hardening
TypeORM query-builder preservation
CI coverage enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@packages/core/src/parser/base.ts`:
- Around line 72-83: Update expandObject to track traversal depth and enforce
the shared MAX_TRAVERSAL_DEPTH limit before recursively processing nested
objects. Reject or stop traversal consistently when the limit is exceeded, while
preserving existing path expansion for valid input; add coverage confirming
inputs deeper than the shared limit are rejected.
In `@packages/typeorm/src/adapter/pagination.ts`:
- Around line 25-31: Update the pagination handling around the queryBuilder.take
and queryBuilder.skip calls to use nullish fallback instead of falsy fallback,
preserving explicit 0 values while still converting null to undefined. Keep the
existing undefined guards and apply the same fix to both this.limit and
this.offset.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad67965d-0521-4a6a-8b43-49610451996b
📒 Files selected for processing (29)
.agents/migration-notes.md.github/workflows/main.ymlpackages/core/src/constants.tspackages/core/src/parser/base.tspackages/core/src/parser/parameter/filters/validate.tspackages/core/src/parser/query.tspackages/core/src/schema/parameter/filters/schema.tspackages/core/src/schema/parameter/filters/types.tspackages/core/src/schema/resolver/module.tspackages/core/test/unit/parser/parameter/filters/validate.spec.tspackages/docs/guide/executing-queries.mdpackages/docs/guide/filters.mdpackages/docs/packages/codec-url.mdpackages/docs/packages/typeorm.mdpackages/parser-expression/src/parameter/filters/module.tspackages/parser-expression/test/unit/parser/filters.spec.tspackages/parser-mongo/src/parameter/filters/module.tspackages/parser-mongo/test/unit/parser/filters.spec.tspackages/parser-simple/src/parameter/filters/module.tspackages/parser-simple/test/unit/parser/filters.spec.tspackages/sql/src/helpers/relation-alias.tspackages/sql/src/visitor/filters.tspackages/sql/test/unit/helpers/relation-alias.spec.tspackages/sql/test/unit/interpreters/regex.spec.tspackages/typeorm/src/adapter/filters.tspackages/typeorm/src/adapter/pagination.tspackages/typeorm/src/adapter/sort.tspackages/typeorm/test/unit/adapter/filters.spec.tspackages/typeorm/test/unit/adapter/module.spec.ts
expandObject enforces the shared MAX_TRAVERSAL_DEPTH across nested objects and dotted keys, so a crafted deeply nested (or cyclic) filter document fails with a typed ParseError instead of overflowing the call stack — mirroring the mongo parser's traversal cap. The typeorm pagination adapter coalesces nullish instead of falsy values: an explicit limit/offset of 0 is applied as a value rather than clearing the builder's take/skip.
Follow-up to #763 / #765: fixes the confirmed findings from a deep code review of 51a906a.
Security / correctness
@rapiq/core):validatehook rejects is now pruned entirely — previously it survived as an emptyFiltersnode, bypassing the schema-defaults fallback (a client could strip a server-mandated default scope by wrapping filters inand()/or()) and crashing schema-aware expression encoding withfilters:compound:empty.$elemMatchinteriors now run through thevalidatehook inside-out; previously interior conditions (e.g. a forbiddenpasswordfilter) bypassed the hook entirely.Validatortype no longer admitsvoid: an inspect-only hook mustreturnthe filter — the previous docs example silently rejected every client filter. Validation also short-circuits when no hook is configured.BaseParser.expandObject: dotted keys and nested objects sharing a prefix ({'realm.id': 1, realm: {name}}) now merge; previously the later key silently replaced the earlier subtree, order-dependently dropping filters.@rapiq/typeorm::rapiq_<n>_<i>); positional:0names could rebind caller-owned bindings (TypeORM parameters are builder-global, last write wins) or a previous run's clauses — silently wrong result sets.ORDER BY/take/skipuntouched, extending the WHERE preservation contract from fix!: harden v2 beta release #763 to all sub-adapters (orderBy({})used to wipe a caller baseline;take(undefined)erased safety caps).@rapiq/sql:regexvalues that are neitherRegExpnor string (e.g. cross-realm RegExps) throw a typedAdapterErrorinstead of being bound raw, matching@rapiq/memory.buildRelationAliasis bounded to 63 chars with an FNV-1a hash suffix — PostgreSQL truncates identifiers at 63 bytes, which could silently collapse long distinct paths back onto one alias.Consistency / infrastructure
MAX_TRAVERSAL_DEPTHconstant exported from core, consumed by the schema resolver and the expression/mongo parsers (the three private copies had already drifted in comparison semantics);buildFiltersDefaultsmoved to core besideapplyFiltersSchemaValidation(was copied in three dialects).prepareQueryContext,parseSource,prepare) so the pipelines can no longer drift.test:coverage: coverage thresholds previously fired for the first time in the release workflow between release-please tagging andnpm publish, stranding a release with tags but no published packages.src/.BREAKING CHANGE:
ValidatorreturnsMaybeAsync<IFilter | undefined>(novoid); relation aliases longer than 63 chars gain a hash suffix; TypeORM filter parameters are namedrapiq_<n>_<i>instead of0,1, ….Summary by CodeRabbit
New Features
$elemMatchfilters.Bug Fixes
Documentation