feat: IP whitelist to bypass bot protection#589
Open
Blaumaus wants to merge 4 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughAdds project-level trusted IP/CIDR whitelists across cloud and community backends, database storage, bot detection, project settings, API types, translations, and documentation. It also centralizes trusted proxy IP resolution. ChangesTrusted IP whitelist
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectSettings
participant ProjectRoute
participant ProjectController
participant ProjectService
participant ProjectStorage
participant BotDetectionService
ProjectSettings->>ProjectRoute: Submit trusted IP/CIDR values
ProjectRoute->>ProjectController: Send normalized ipWhitelist
ProjectController->>ProjectService: Validate and update project
ProjectService->>ProjectStorage: Store ipWhitelist
BotDetectionService->>ProjectStorage: Read project whitelist
BotDetectionService-->>BotDetectionService: Return NEGATIVE for matching IP
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/apps/community/src/project/project.service.ts (1)
636-655: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMissing
validateIPWhitelistin communityvalidateProject.The cloud edition's
validateProjectcallsthis.validateIPWhitelist(projectDTO)(validating total length ≤ 300 chars and each entry as a valid IP/CIDR), but the community edition omits both the method and the call. The community controller only checks array type and a 1000-item cap — it does not validate IP format. Invalid entries (e.g.,not-an-ip) could be persisted and later cause unexpected behavior inisIpInRangeduring bot detection.🔒 Proposed fix: add `validateIPWhitelist` and wire it into `validateProject`
validateIPBlacklist(projectDTO: ProjectDTO | UpdateProjectDto) { if (_size(_join(projectDTO.ipBlacklist, ',')) > 300) throw new UnprocessableEntityException( 'The list of allowed blacklisted IP addresses must be less than 300 characters.', ) _map(projectDTO.ipBlacklist, (ip) => { if (!net.isIP(_trim(ip)) && !IP_REGEX.test(_trim(ip))) { throw new ConflictException(`IP address ${ip} is not correct`) } }) } + validateIPWhitelist(projectDTO: ProjectDTO | UpdateProjectDto) { + if (_size(_join(projectDTO.ipWhitelist, ',')) > 300) + throw new UnprocessableEntityException( + 'The list of whitelisted IP addresses must be less than 300 characters.', + ) + _map(projectDTO.ipWhitelist, (ip) => { + if (!net.isIP(_trim(ip)) && !IP_REGEX.test(_trim(ip))) { + throw new ConflictException(`IP address ${ip} is not correct`) + } + }) + } + validateCountryBlacklist(projectDTO: ProjectDTO | UpdateProjectDto) {Then update
validateProject:this.validateOrigins(projectDTO) this.validateIPBlacklist(projectDTO) + this.validateIPWhitelist(projectDTO) this.validateCountryBlacklist(projectDTO)🤖 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 `@backend/apps/community/src/project/project.service.ts` around lines 636 - 655, Add a community-side validateIPWhitelist method matching the cloud validation rules: enforce a maximum total length of 300 characters and validate every whitelist entry as a valid IP or CIDR, rejecting invalid values with the established validation exception. Invoke this method from validateProject alongside validateIPBlacklist and validateCountryBlacklist for non-creating projects.
🧹 Nitpick comments (2)
web/app/pages/Project/Settings/ProjectSettings.tsx (1)
74-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReused
MAX_IPBLACKLIST_LENGTHconstant foripWhitelistvalidation.The constant name implies it's scoped to
ipBlacklist, but it's now also used to validateipWhitelist(Line 737). Functionally fine since both limits are 300 chars, but a rename (e.g.MAX_IP_LIST_LENGTH) would better reflect its dual usage and avoid confusion if the limits ever diverge.♻️ Suggested rename
-const MAX_IPBLACKLIST_LENGTH = 300 +const MAX_IP_LIST_LENGTH = 300Then update both usages (Lines 731-735 and 737-741) accordingly.
Also applies to: 731-741
🤖 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 `@web/app/pages/Project/Settings/ProjectSettings.tsx` at line 74, Rename MAX_IPBLACKLIST_LENGTH to a neutral shared name such as MAX_IP_LIST_LENGTH in ProjectSettings, and update both the ipBlacklist and ipWhitelist validation usages to reference the renamed constant.backend/migrations/clickhouse/initialise_selfhosted.js (1)
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: CODEC inconsistency between init script and migration.
The init script defines
ipWhitelist Nullable(String) CODEC(ZSTD(3))while the migration fileselfhosted_2026_07_12_ip_whitelist.jsusesNullable(String) DEFAULT NULLwithoutCODEC(ZSTD(3)). Existing installations won't get column-level ZSTD compression on this column. Consider addingCODEC(ZSTD(3))to the migration for consistency.🤖 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 `@backend/migrations/clickhouse/initialise_selfhosted.js` at line 30, Update the selfhosted_2026_07_12_ip_whitelist migration to define the ipWhitelist column with CODEC(ZSTD(3)), matching the initialization schema while preserving its Nullable(String) and DEFAULT NULL behavior.
🤖 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 `@backend/apps/cloud/src/common/utils.ts`:
- Around line 443-456: Update the caller flow around the X-Forwarded-For parsing
loop to reject forwarded headers when realIp is absent: return null so callers
fall back to the socket/request IP. Only walk hops and return a non-trusted
candidate after confirming realIp identifies a trusted connection peer; preserve
the existing trusted-peer parsing behavior and realIp fallback otherwise.
In `@backend/apps/cloud/src/project/dto/create-project.dto.ts`:
- Around line 52-55: Update ProjectController.create and its project
construction flow to retain the request’s ipWhitelist, and ensure
validateProject(..., true) still performs whitelist validation instead of
returning before it. Preserve existing project creation behavior while rejecting
invalid whitelist values and persisting valid ones.
In `@backend/apps/cloud/src/project/dto/project.dto.ts`:
- Around line 56-62: Update the ApiProperty example for ipWhitelist in the
project DTO to use an array of individual IP addresses and CIDR ranges, matching
its string[] type instead of a comma-separated string.
In `@backend/apps/cloud/src/project/project.service.ts`:
- Around line 794-806: Update validateIPWhitelist to first verify that
projectDTO.ipWhitelist is an array, then enforce the existing 300-character
joined-length limit before calling _join or _map. Preserve the current IP
validation behavior for valid arrays and reject non-array payloads using the
service’s existing validation exception pattern.
In `@backend/apps/community/src/project/project.controller.ts`:
- Around line 1100-1116: The ipWhitelist update path in validateProject() only
checks array shape and size, allowing invalid IP/CIDR entries to persist. Reuse
the existing cloud validateIPWhitelist() validation before assigning
project.ipWhitelist, preserving the current empty-list, array, and maximum-count
handling while rejecting invalid entries before persistence.
In `@web/app/routes/projects.settings`.$id.tsx:
- Line 127: Update the ipWhitelist handling in the route’s form submission logic
to split the input into entries, trim each value, and remove empty entries
before sending it to the backend. Apply the same normalization at the additional
ipWhitelist handling around the later referenced block, while preserving the
ability to submit an empty whitelist when no valid entries remain.
---
Outside diff comments:
In `@backend/apps/community/src/project/project.service.ts`:
- Around line 636-655: Add a community-side validateIPWhitelist method matching
the cloud validation rules: enforce a maximum total length of 300 characters and
validate every whitelist entry as a valid IP or CIDR, rejecting invalid values
with the established validation exception. Invoke this method from
validateProject alongside validateIPBlacklist and validateCountryBlacklist for
non-creating projects.
---
Nitpick comments:
In `@backend/migrations/clickhouse/initialise_selfhosted.js`:
- Line 30: Update the selfhosted_2026_07_12_ip_whitelist migration to define the
ipWhitelist column with CODEC(ZSTD(3)), matching the initialization schema while
preserving its Nullable(String) and DEFAULT NULL behavior.
In `@web/app/pages/Project/Settings/ProjectSettings.tsx`:
- Line 74: Rename MAX_IPBLACKLIST_LENGTH to a neutral shared name such as
MAX_IP_LIST_LENGTH in ProjectSettings, and update both the ipBlacklist and
ipWhitelist validation usages to reference the renamed constant.
🪄 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: 041e5402-6a9e-4417-aa34-19c2d671c3a8
📒 Files selected for processing (27)
backend/.env.examplebackend/apps/cloud/src/analytics/bot-detection.service.tsbackend/apps/cloud/src/analytics/protection/analytics-read.util.tsbackend/apps/cloud/src/common/utils.tsbackend/apps/cloud/src/project/dto/create-project.dto.tsbackend/apps/cloud/src/project/dto/project.dto.tsbackend/apps/cloud/src/project/entity/project.entity.tsbackend/apps/cloud/src/project/project.controller.tsbackend/apps/cloud/src/project/project.service.tsbackend/apps/community/src/analytics/bot-detection.service.tsbackend/apps/community/src/common/utils.tsbackend/apps/community/src/project/dto/project.dto.tsbackend/apps/community/src/project/entity/project.entity.tsbackend/apps/community/src/project/project.controller.tsbackend/apps/community/src/project/project.service.tsbackend/migrations/clickhouse/initialise_selfhosted.jsbackend/migrations/clickhouse/selfhosted_2026_07_12_ip_whitelist.jsbackend/migrations/mysql/2026_07_12_ip_whitelist.sqldocs/content/docs/api/admin.mdxdocs/content/docs/sitesettings/bot-protection.mdxdocs/content/docs/sitesettings/project-configuration.mdxweb/app/api/api.server.tsweb/app/lib/models/Project.tsweb/app/pages/Project/Settings/ProjectSettings.tsxweb/app/pages/Project/Settings/tabs/Shields.tsxweb/app/routes/projects.settings.$id.tsxweb/public/locales/en.json
Comment on lines
+52
to
+55
| @ApiProperty({ | ||
| required: false, | ||
| }) | ||
| ipWhitelist?: string[] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Persist and validate ipWhitelist during project creation.
ProjectController.create calls validateProject(..., true), which returns before whitelist validation, and the constructed project object omits ipWhitelist. Create requests therefore silently discard this field while accepting invalid values.
🤖 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 `@backend/apps/cloud/src/project/dto/create-project.dto.ts` around lines 52 -
55, Update ProjectController.create and its project construction flow to retain
the request’s ipWhitelist, and ensure validateProject(..., true) still performs
whitelist validation instead of returning before it. Preserve existing project
creation behavior while rejecting invalid whitelist values and persisting valid
ones.
| const password = formData.get('password')?.toString() | ||
| const origins = formData.get('origins')?.toString() | ||
| const ipBlacklist = formData.get('ipBlacklist')?.toString() | ||
| const ipWhitelist = formData.get('ipWhitelist')?.toString() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Filter empty whitelist entries before submitting.
Trailing commas, repeated commas, or whitespace-only input produce '' entries. The backend validates every entry and rejects those values, so users cannot reliably save or clear the field.
Proposed fix
- if (ipWhitelist !== undefined)
- updateData.ipWhitelist = ipWhitelist
- ? ipWhitelist.split(',').map((ip) => ip.trim())
- : null
+ if (ipWhitelist !== undefined) {
+ const values = ipWhitelist
+ .split(',')
+ .map((ip) => ip.trim())
+ .filter(Boolean)
+ updateData.ipWhitelist = values.length > 0 ? values : null
+ }Also applies to: 182-185
🤖 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 `@web/app/routes/projects.settings`.$id.tsx at line 127, Update the ipWhitelist
handling in the route’s form submission logic to split the input into entries,
trim each value, and remove empty entries before sending it to the backend.
Apply the same normalization at the additional ipWhitelist handling around the
later referenced block, while preserving the ability to submit an empty
whitelist when no valid entries remain.
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
2 participants
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.
Changes
If applicable, please describe what changes were made in this pull request.
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit