fix: enforce maxFilterValues on subscription and count filters - #771
Merged
Conversation
Filters whose array criteria (ids, authors, kinds, #<tag>) hold more than limits.client.subscription.maxFilterValues values in total are now rejected with 'Too many filter values' instead of being passed to PostgreSQL as an unbounded WHERE IN (...). The setting was defined in the default settings and surfaced in the admin settings editor while nothing read it, so a client could send a filter with thousands of values and make the database build a hash table for it. The enforced limit is also advertised in the NIP-11 limitation object as max_filter_values, a non-standard field since NIP-11 has no field for per-filter value counts. Also renames test/unit/utils/filter.ts to filter.spec.ts so its existing assertions are picked up by the unit test glob.
🦋 Changeset detectedLatest commit: 7a0cfbe The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Collaborator
|
coverage: 72.29% (+0.06%) from 72.235% — fix/enforce-max-filter-values into main |
There was a problem hiding this comment.
🟢 Approval recommended
The reviewed changes are covered by tests and have no unresolved blocking issues.
Pull request overview
Enforces maxFilterValues for REQ and COUNT filters before repository access, while documenting and advertising the limit.
Changes:
- Counts array-based filter values and rejects oversized filters.
- Adds NIP-11 and configuration documentation.
- Adds focused tests and a changeset.
File summaries
| File | Description |
|---|---|
test/unit/utils/filter.ts |
Renamed to the executable test filename. |
test/unit/utils/filter.spec.ts |
Tests filter-value counting. |
test/unit/handlers/subscribe-message-handler.spec.ts |
Tests REQ enforcement. |
test/unit/handlers/request-handlers/root-request-handler.spec.ts |
Tests NIP-11 advertisement. |
test/unit/handlers/count-message-handler.spec.ts |
Tests COUNT enforcement. |
src/utils/filter.ts |
Counts array-valued filter criteria. |
src/handlers/subscribe-message-handler.ts |
Rejects oversized REQ filters. |
src/handlers/request-handlers/root-request-handler.ts |
Advertises max_filter_values. |
src/handlers/count-message-handler.ts |
Rejects oversized COUNT filters. |
CONFIGURATION.md |
Documents maxFilterValues. |
.changeset/enforce-max-filter-values.md |
Records the release change. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
limits.client.subscription.maxFilterValueswas defined in the default settings (2500) and surfaced in the admin settings editor, but nothing read it:EventRepository.findByFilters()passed a client'sauthors/ids/#e/#parrays straight intoWHERE IN (...). A filter with 5,000 values was accepted and PostgreSQL had to build a hash table for it — a cheap way to spike CPU and memory per request.Changes
SubscribeMessageHandler.canSubscribe(REQ) andCountMessageHandler.canCount(COUNT) now reject a filter whose array criteria hold more thanmaxFilterValuesvalues in total:Too many filter values: Number of values per filter must be less than or equal to N. Zero (or unset) keeps the check disabled, matching how the neighbouringlimits.client.subscriptionsettings behave.countFilterValues()insrc/utils/filter.tssums a filter's array criteria (ids,authors,kinds,#<tag>, ...) — scalar criteria (since,until,limit,search) are not values, and are bounded by their own settings.max_filter_values. NIP-11 has no field for per-filter value counts, so this is a non-standard extension; thelimitationobject already carries non-standard keys such asdefault_limitandsearch_supported.max_event_tagsis deliberately left alone — that is NIP-11's tags-per-event field and is unrelated to this setting.CONFIGURATION.mdgains the missingmaxFilterValuesrow.Both entry points are covered because both hand client filters to the repository: REQ streams via
findByFilters, COUNT viacountByFilters. No other code path takes client-supplied filters.Verification
pnpm run test:unit— 1864 passing (the pre-commit hook runs it, along withbiome lintandtsc -p tsconfig.build.json, all clean).One thing to flag rather than sneak in: this also renames
test/unit/utils/filter.tstofilter.spec.ts, because the unit glob istest/**/*.spec.tsand itsisGenericTagQueryassertions had never been running. Happy to split that out into its own PR if you prefer this one minimal.Closes #599