Skip to content

Exclude ips#527

Merged
goldflag merged 9 commits intomasterfrom
exclude-ips
Aug 3, 2025
Merged

Exclude ips#527
goldflag merged 9 commits intomasterfrom
exclude-ips

Conversation

@goldflag
Copy link
Copy Markdown
Collaborator

@goldflag goldflag commented Aug 3, 2025

Summary by CodeRabbit

  • New Features

    • Added the ability for site administrators to manage a list of excluded IP addresses via the Site Settings UI.
    • Introduced a dedicated IP Exclusion Manager component for adding, editing, and validating excluded IPs with real-time feedback and error handling.
    • Implemented new API endpoints to fetch and update excluded IPs for each site.
  • Improvements

    • Session replay and event tracking now respect excluded IP lists, ensuring events from specified IPs are not recorded or tracked.
    • Added comprehensive IP pattern validation supporting IPv4, IPv6, CIDR notation, and IPv4 ranges.
  • Bug Fixes

    • Improved error handling and user feedback for invalid IP patterns in the exclusion list.
  • Chores

    • Updated dependencies to include the "ip-address" library for enhanced IP parsing and validation.
    • Replaced console logs with structured logging for better monitoring and debugging.
    • Enhanced logging format for clearer and more concise log output.

Implements IP exclusion functionality to allow administrators to exclude traffic from specific IP addresses or ranges from being tracked.

This includes:
- Adding API endpoints to manage excluded IPs for a site.
- Creating a UI component to manage IP exclusions in site settings.
- Updating the tracker to check if an IP address is excluded before tracking an event.
- Includes validation for IP addresses and ranges.
Enhances IP exclusion management by implementing robust validation and error handling.

The changes include:

- Converts site ID to string type in API calls for consistency.
- Adds client-side IP validation logic, mirroring server-side validation, and provides more descriptive error messages for invalid patterns, including IPv6 range restrictions.
- Improves error handling by parsing JSON error responses from the server, providing more specific feedback to the user.
- Refactors the IP validation logic into a separate module for better code organization and reusability.
- Enforces authentication and authorization checks on the server-side to secure IP exclusion updates.
- Implements atomic updates for IP exclusion changes, updating both database and cache within a transaction.
Adjusts IPv6 validation logic to correctly handle compressed notation.

Ensures that the sum of parts before and after the double colon is strictly less than 8, guaranteeing at least one zero group is replaced.
Implements client-side IP address validation using the `ip-address` library.

This change introduces a new utility for validating IP addresses, CIDR notations, and IP ranges, mirroring the server-side validation logic. It enhances user experience by providing immediate feedback on invalid IP formats.

Supports IPv4 and IPv6 addresses, including CIDR notation.  Range notation is supported for IPv4 addresses only.
@vercel
Copy link
Copy Markdown

vercel bot commented Aug 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rybbit ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 3, 2025 11:05pm

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Aug 3, 2025

Walkthrough

This update introduces a full-stack feature for managing excluded IP addresses per site. It adds backend database schema changes, API endpoints, server-side validation, and IP exclusion logic for event/session recording. On the frontend, new UI components, API hooks, and client-side validation utilities are implemented, allowing users to view and edit excluded IPs. Supporting dependencies are added to both client and server.

Changes

Cohort / File(s) Change Summary
Client: Dependency Addition
client/package.json
Adds ip-address npm package as a dependency.
Server: Dependency Addition
server/package.json
Adds ip-address npm package as a dependency.
Client: API & Hooks for Excluded IPs
client/src/api/admin/excludedIPs.ts
Introduces API functions and React Query hooks for fetching/updating excluded IPs per site.
Client: IP Exclusion UI
client/src/components/SiteSettings/IPExclusionManager.tsx
Adds a new React component for managing a site's excluded IPs, with validation and UI controls.
Client: Site Settings Integration
client/src/components/SiteSettings/SiteConfiguration.tsx
Integrates the IP Exclusion Manager into the site settings page.
Client: IP Validation Utility
client/src/lib/ipValidation.ts
Implements client-side IP pattern validation logic matching server rules.
Server: DB Schema
server/src/db/postgres/schema.ts
Adds an excludedIPs JSONB column to the sites table.
Server: Site Config Cache
server/src/lib/siteConfig.ts
Extends site config to cache, retrieve, and update excluded IPs per site.
Server: IP Utils
server/src/lib/ipUtils.ts
New module for matching and validating IPs against exclusion patterns (single, CIDR, range).
Server: Add Site
server/src/api/sites/addSite.ts
Ensures new sites have excludedIPs and apiKey in the config cache.
Server: Get Excluded IPs API
server/src/api/sites/getSiteExcludedIPs.ts
Adds API handler to fetch a site's excluded IPs, with validation and auth.
Server: Update Excluded IPs API
server/src/api/sites/updateSiteExcludedIPs.ts
Adds API handler to update a site's excluded IPs, with validation, DB update, and cache sync.
Server: API Routing
server/src/index.ts
Registers new API endpoints for getting and updating excluded IPs.
Server: Event Tracking IP Exclusion
server/src/services/tracker/trackEvent.ts
Skips event tracking for requests from excluded IPs, with logging.
Server: Session Replay IP Exclusion
server/src/api/sessionReplay/recordSessionReplay.ts
Skips session replay recording for requests from excluded IPs, with logging.
Server: Logging Improvements
server/src/api/admin/getAdminOrganizations.ts, server/src/db/geolocation/geolocation.ts
Replaces console logs with structured logger calls.
Server: Test Updates
server/src/services/shared/requestValidation.test.ts
Adds excludedIPs to mock site config interface; standardizes string quoting.
Server: Logger Configuration
server/src/lib/logger/logger.ts
Updates logger formatting options for timestamps and output style.

Sequence Diagram(s)

Excluded IPs Update Flow (Frontend to Backend)

sequenceDiagram
    participant User
    participant IPExclusionManager (Client)
    participant API (Client)
    participant Server API
    participant DB
    participant SiteConfig Cache

    User->>IPExclusionManager (Client): Edit IP list, click Save
    IPExclusionManager (Client)->>API (Client): POST /api/site/:siteId/excluded-ips
    API (Client)->>Server API: POST /api/site/:siteId/excluded-ips
    Server API->>DB: Update excludedIPs column for site
    Server API->>SiteConfig Cache: Update in-memory excludedIPs
    Server API-->>API (Client): Success/Failure response
    API (Client)-->>IPExclusionManager (Client): Show toast, update UI
Loading

Event Tracking with IP Exclusion

sequenceDiagram
    participant Client
    participant Server trackEvent
    participant SiteConfig
    participant IPUtils

    Client->>Server trackEvent: Send event with IP
    Server trackEvent->>SiteConfig: Get excludedIPs for site
    Server trackEvent->>IPUtils: isIPExcluded(ip, excludedIPs)
    alt IP is excluded
        Server trackEvent-->>Client: 200 OK, event not tracked
    else IP not excluded
        Server trackEvent->>...: Proceed with event tracking
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

A hop, a skip, a bunny dash—
We block some IPs in a flash!
With patterns checked and lists in tow,
No sneaky packets now can flow.
The UI’s bright, the backend’s tough,
This bunny’s code is IP-proofed enough!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 309f556 and c4c2253.

📒 Files selected for processing (4)
  • client/src/components/SiteSettings/IPExclusionManager.tsx (1 hunks)
  • server/src/api/sessionReplay/recordSessionReplay.ts (5 hunks)
  • server/src/lib/siteConfig.ts (5 hunks)
  • server/src/services/tracker/trackEvent.ts (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • server/src/services/tracker/trackEvent.ts
  • server/src/api/sessionReplay/recordSessionReplay.ts
  • client/src/components/SiteSettings/IPExclusionManager.tsx
🧰 Additional context used
📓 Path-based instructions (2)
{client,server}/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (CLAUDE.md)

{client,server}/**/*.{ts,tsx}: Use TypeScript with strict typing throughout both client and server
Use try/catch blocks with specific error types for error handling
Use camelCase for variables and functions, PascalCase for components and types
Group imports by external, then internal, and sort alphabetically within groups

Files:

  • server/src/lib/siteConfig.ts
server/**/*

📄 CodeRabbit Inference Engine (CLAUDE.md)

Backend: Use Fastify, Drizzle ORM (Postgres), ClickHouse, and Zod

Files:

  • server/src/lib/siteConfig.ts
🧠 Learnings (1)
📚 Learning: applies to server/**/* : backend: use fastify, drizzle orm (postgres), clickhouse, and zod...
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T17:30:25.547Z
Learning: Applies to server/**/* : Backend: Use Fastify, Drizzle ORM (Postgres), ClickHouse, and Zod

Applied to files:

  • server/src/lib/siteConfig.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Client Images (ubuntu-24.04-arm, linux/arm64)
  • GitHub Check: Build Client Images (ubuntu-latest, linux/amd64)
  • GitHub Check: Build Backend Images (ubuntu-24.04-arm, linux/arm64)
🔇 Additional comments (7)
server/src/lib/siteConfig.ts (7)

3-4: LGTM: Import statements are properly structured.

The new imports for logger and IP utility functions are correctly added and follow the coding guidelines for internal import grouping.


12-12: LGTM: Interface update follows TypeScript guidelines.

The excludedIPs: string[] property addition is properly typed and consistent with the interface design.


29-29: LGTM: Database loading includes proper safety checks.

The database field selection and safe array handling with Array.isArray() check prevents runtime errors and follows the established pattern for other properties.

Also applies to: 44-44


160-167: LGTM: Getter method follows established patterns.

The getExcludedIPs method is consistent with other getter methods in the class, includes proper TypeScript typing, and has null-safe fallback handling.


169-178: LGTM: Update method follows established patterns.

The updateSiteExcludedIPs method is consistent with other update methods, includes proper config existence checks, and follows TypeScript typing guidelines.


203-219: LGTM: IP exclusion logic is well-structured.

The isIPExcluded method follows TypeScript guidelines, includes early return optimization, and properly separates concerns with the helper method. The logic is clear and handles the common case efficiently.


221-252: IPv6 Range Limitation Is Consistently Handled and Documented

The matchesIPPattern method in siteConfig.ts aligns with the utility implementations in ipUtils.ts and ipValidation.ts, where IPv6 range notation is explicitly rejected, logged, and documented. No further changes are needed unless you decide to add IPv6 range support.

Key references:

  • server/src/lib/ipUtils.ts
    • Logs and rejects IPv6 ranges with a warning:
    “IPv6 range notation not supported. Use CIDR notation instead (e.g., 2001:db8::/32)”
  • client/src/lib/ipValidation.ts
    • Mirrors the same validation behavior and documentation.
  • server/src/lib/siteConfig.ts
    • JSDoc clearly states “IPv4 only, IPv6 ranges not supported.”

LGTM—approving as-is.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch exclude-ips

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (5)
server/src/api/sites/updateSiteExcludedIPs.ts (2)

1-8: Sort imports alphabetically within groups

According to the coding guidelines, imports should be sorted alphabetically within their groups.

 import { FastifyReply, FastifyRequest } from "fastify";
 import { z } from "zod";
+import { eq } from "drizzle-orm";
+import { getUserHasAccessToSite } from "../../lib/auth-utils.js";
+import { validateIPPattern } from "../../lib/ipUtils.js";
+import { siteConfig } from "../../lib/siteConfig.js";
 import { db } from "../../db/postgres/postgres.js";
 import { sites } from "../../db/postgres/schema.js";
-import { siteConfig } from "../../lib/siteConfig.js";
-import { validateIPPattern } from "../../lib/ipUtils.js";
-import { getUserHasAccessToSite } from "../../lib/auth-utils.js";
-import { eq } from "drizzle-orm";

27-36: Simplify siteId validation

The isNaN check is redundant since Number.isInteger(NaN) returns false. Consider moving this validation to the Zod schema level.

Update the schema:

 const updateExcludedIPsSchema = z.object({
-  siteId: z.string().min(1),
+  siteId: z.coerce.number().int().positive(),
   excludedIPs: z.array(z.string().trim().min(1)).max(100),
 });

Then simplify this section:

-    const { siteId, excludedIPs } = validationResult.data;
-    const numericSiteId = Number(siteId);
-
-    // Validate that siteId is a valid integer
-    if (!Number.isInteger(numericSiteId) || isNaN(numericSiteId) || numericSiteId <= 0) {
-      return reply.status(400).send({
-        success: false,
-        error: "Invalid site ID: must be a positive integer",
-      });
-    }
+    const { siteId: numericSiteId, excludedIPs } = validationResult.data;
client/src/components/SiteSettings/IPExclusionManager.tsx (2)

3-12: Sort imports alphabetically within groups

Imports should be sorted alphabetically within their groups according to the coding guidelines.

-import { Minus, Plus } from "lucide-react";
 import React, { useState } from "react";
+import { Minus, Plus } from "lucide-react";
 import { toast } from "sonner";

+import { useGetExcludedIPs, useUpdateExcludedIPs } from "@/api/admin/excludedIPs";
 import { Button } from "@/components/ui/button";
 import { Input } from "@/components/ui/input";
 import { Label } from "@/components/ui/label";
-
-import { useGetExcludedIPs, useUpdateExcludedIPs } from "@/api/admin/excludedIPs";
 import { validateIPPattern } from "@/lib/ipValidation";

23-34: Consider using derived state instead of useEffect

According to the coding guidelines, React components should use minimal useEffect. Consider deriving the initial state during render instead.

-  const [ipList, setIpList] = useState<string[]>([]);
+  const initialIpList = excludedIPsData?.excludedIPs && excludedIPsData.excludedIPs.length > 0 
+    ? excludedIPsData.excludedIPs 
+    : [""];
+  const [ipList, setIpList] = useState<string[]>(initialIpList);
   const [hasUnsavedChanges, setHasUnsavedChanges] = useState(false);
-
-  // Initialize IP list when data is loaded
-  React.useEffect(() => {
-    if (excludedIPsData?.excludedIPs) {
-      setIpList(excludedIPsData.excludedIPs.length > 0 ? excludedIPsData.excludedIPs : [""]);
-      setHasUnsavedChanges(false);
-    } else if (!isLoading) {
-      setIpList([""]);
-    }
-  }, [excludedIPsData, isLoading]);

Note: This approach might need additional logic to handle data updates if the component doesn't remount when siteId changes.

server/src/lib/ipUtils.ts (1)

1-193: Consider extracting shared validation logic

The IP validation logic is duplicated between client and server. While the current implementation maintains consistency, consider extracting this into a shared package to ensure they stay synchronized.

Options:

  1. Create a shared TypeScript package that both client and server can import
  2. Generate client validation from server validation using code generation
  3. Add integration tests to verify client-server validation consistency
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3958491 and 27b5c96.

⛔ Files ignored due to path filters (2)
  • client/package-lock.json is excluded by !**/package-lock.json
  • server/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (15)
  • client/package.json (1 hunks)
  • client/src/api/admin/excludedIPs.ts (1 hunks)
  • client/src/components/SiteSettings/IPExclusionManager.tsx (1 hunks)
  • client/src/components/SiteSettings/SiteConfiguration.tsx (2 hunks)
  • client/src/lib/ipValidation.ts (1 hunks)
  • server/package.json (1 hunks)
  • server/src/api/sessionReplay/recordSessionReplay.ts (2 hunks)
  • server/src/api/sites/addSite.ts (1 hunks)
  • server/src/api/sites/getSiteExcludedIPs.ts (1 hunks)
  • server/src/api/sites/updateSiteExcludedIPs.ts (1 hunks)
  • server/src/db/postgres/schema.ts (1 hunks)
  • server/src/index.ts (2 hunks)
  • server/src/lib/ipUtils.ts (1 hunks)
  • server/src/lib/siteConfig.ts (4 hunks)
  • server/src/services/tracker/trackEvent.ts (6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
client/**/*

📄 CodeRabbit Inference Engine (CLAUDE.md)

Frontend: Use Next.js, Tailwind CSS, Shadcn UI, Tanstack Query, Zustand, Luxon, Nivo, and react-hook-form

Files:

  • client/package.json
  • client/src/lib/ipValidation.ts
  • client/src/components/SiteSettings/SiteConfiguration.tsx
  • client/src/api/admin/excludedIPs.ts
  • client/src/components/SiteSettings/IPExclusionManager.tsx
server/**/*

📄 CodeRabbit Inference Engine (CLAUDE.md)

Backend: Use Fastify, Drizzle ORM (Postgres), ClickHouse, and Zod

Files:

  • server/package.json
  • server/src/db/postgres/schema.ts
  • server/src/api/sites/addSite.ts
  • server/src/lib/siteConfig.ts
  • server/src/api/sites/updateSiteExcludedIPs.ts
  • server/src/services/tracker/trackEvent.ts
  • server/src/api/sites/getSiteExcludedIPs.ts
  • server/src/index.ts
  • server/src/api/sessionReplay/recordSessionReplay.ts
  • server/src/lib/ipUtils.ts
{client,server}/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (CLAUDE.md)

{client,server}/**/*.{ts,tsx}: Use TypeScript with strict typing throughout both client and server
Use try/catch blocks with specific error types for error handling
Use camelCase for variables and functions, PascalCase for components and types
Group imports by external, then internal, and sort alphabetically within groups

Files:

  • server/src/db/postgres/schema.ts
  • client/src/lib/ipValidation.ts
  • client/src/components/SiteSettings/SiteConfiguration.tsx
  • server/src/api/sites/addSite.ts
  • server/src/lib/siteConfig.ts
  • server/src/api/sites/updateSiteExcludedIPs.ts
  • server/src/services/tracker/trackEvent.ts
  • server/src/api/sites/getSiteExcludedIPs.ts
  • server/src/index.ts
  • client/src/api/admin/excludedIPs.ts
  • server/src/api/sessionReplay/recordSessionReplay.ts
  • client/src/components/SiteSettings/IPExclusionManager.tsx
  • server/src/lib/ipUtils.ts
🧠 Learnings (4)
📚 Learning: applies to client/**/* : frontend: use next.js, tailwind css, shadcn ui, tanstack query, zustand, lu...
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T17:30:25.547Z
Learning: Applies to client/**/* : Frontend: Use Next.js, Tailwind CSS, Shadcn UI, Tanstack Query, Zustand, Luxon, Nivo, and react-hook-form

Applied to files:

  • client/package.json
  • server/package.json
  • client/src/api/admin/excludedIPs.ts
  • client/src/components/SiteSettings/IPExclusionManager.tsx
📚 Learning: applies to server/**/* : backend: use fastify, drizzle orm (postgres), clickhouse, and zod...
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T17:30:25.547Z
Learning: Applies to server/**/* : Backend: Use Fastify, Drizzle ORM (Postgres), ClickHouse, and Zod

Applied to files:

  • server/package.json
  • server/src/api/sites/updateSiteExcludedIPs.ts
  • server/src/api/sites/getSiteExcludedIPs.ts
  • server/src/api/sessionReplay/recordSessionReplay.ts
📚 Learning: applies to {client,server}/**/*.{ts,tsx} : group imports by external, then internal, and sort alphab...
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T17:30:25.547Z
Learning: Applies to {client,server}/**/*.{ts,tsx} : Group imports by external, then internal, and sort alphabetically within groups

Applied to files:

  • client/src/components/SiteSettings/SiteConfiguration.tsx
📚 Learning: applies to client/**/*.{tsx} : client: use react functional components with minimal useeffect and in...
Learnt from: CR
PR: rybbit-io/rybbit#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T17:30:25.547Z
Learning: Applies to client/**/*.{tsx} : Client: Use React functional components with minimal useEffect and inline functions

Applied to files:

  • client/src/components/SiteSettings/IPExclusionManager.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build Client Images (ubuntu-latest, linux/amd64)
  • GitHub Check: Build Client Images (ubuntu-24.04-arm, linux/arm64)
🔇 Additional comments (38)
server/package.json (1)

40-40: ip-address dependency version verified and secure

  • Confirmed that version 10.0.1 is the latest release on npm.
  • No known security advisories detected for this package.
client/package.json (1)

56-56: LGTM! Consistent dependency addition.

The ip-address dependency addition is consistent with the server-side package.json, ensuring compatibility between client and server IP validation logic.

server/src/db/postgres/schema.ts (1)

66-66: Well-designed schema addition.

The excludedIPs column addition is well-implemented:

  • Uses jsonb type which is efficient for PostgreSQL JSON operations
  • Defaults to empty array to prevent null handling issues
  • Clear comment explains the column's purpose
  • Follows consistent naming conventions
server/src/api/sites/addSite.ts (1)

105-107: Good defensive programming for cache updates.

The additions to the siteConfig cache are well-implemented:

  • Array.isArray() check with fallback to empty array provides type safety
  • Consistent with the database schema default value
  • Both excludedIPs and apiKey follow the existing pattern for cache updates
client/src/components/SiteSettings/SiteConfiguration.tsx (2)

34-34: Import follows proper grouping guidelines.

The import is correctly placed in the internal imports group and follows alphabetical ordering within the group.


218-220: Clean UI integration of IP Exclusion Manager.

The IPExclusionManager component is well-integrated:

  • Logical placement between Bot Blocking and Domain Settings
  • Appropriate props passed (siteId and disabled)
  • Maintains consistent spacing and section structure
  • Doesn't disrupt existing functionality
server/src/index.ts (2)

54-54: LGTM! New import additions follow existing patterns.

The imports for getSiteExcludedIPs and updateSiteExcludedIPs are properly placed in alphabetical order within the sites section and follow the established naming conventions.

Also applies to: 59-59


358-359: LGTM! Route registration follows REST conventions.

The new routes correctly implement RESTful patterns:

  • GET /api/site/:siteId/excluded-ips for retrieving excluded IPs
  • POST /api/site/:siteId/excluded-ips for updating excluded IPs

The placement in the administrative section is appropriate.

server/src/api/sessionReplay/recordSessionReplay.ts (2)

3-4: LGTM! Clean import additions for IP exclusion functionality.

The imports for isIPExcluded and siteConfig are appropriately added to support the new IP exclusion feature.


89-102: Well-implemented IP exclusion logic with proper placement.

The IP exclusion check is strategically placed after authentication/authorization but before processing, which is optimal for performance and security. The implementation correctly:

  • Ensures site config is initialized
  • Retrieves the request IP using the existing utility
  • Gets excluded IPs from the cached site config (efficient)
  • Returns a successful response when excluded (maintains client compatibility)
  • Includes appropriate logging for monitoring
server/src/services/tracker/trackEvent.ts (2)

5-5: LGTM! Import addition for IP exclusion functionality.

The import for isIPExcluded is appropriately added to support the new IP exclusion feature.


176-179: Good improvement to logger consistency.

The formatting improvements to the logger calls enhance consistency and readability by providing structured context objects with clear field names.

Also applies to: 191-191, 205-208

server/src/lib/siteConfig.ts (3)

10-10: LGTM! Clean interface extension for IP exclusion.

The addition of excludedIPs: string[] to the SiteConfigData interface properly extends the configuration structure for the new feature.


27-27: Robust database integration with defensive programming.

The implementation correctly:

  • Selects the excludedIPs field from the database
  • Uses Array.isArray() check with fallback to empty array for safety
  • Handles potentially inconsistent database data gracefully

Also applies to: 42-42


158-177: Well-designed methods following established patterns.

Both new methods maintain consistency with the existing codebase:

  • getExcludedIPs(): Proper type conversion and null-safe return
  • updateSiteExcludedIPs(): Follows the established update pattern with existence check
  • Both handle edge cases appropriately (missing config, type conversion)
server/src/api/sites/getSiteExcludedIPs.ts (4)

1-11: Excellent setup following established patterns.

The file structure properly follows the coding guidelines:

  • Clean imports with Fastify, Zod, and Drizzle ORM
  • Well-defined Zod schema for parameter validation
  • TypeScript with strict typing throughout

14-33: Comprehensive input validation with proper error handling.

The validation logic is robust:

  • Uses Zod for initial parameter parsing
  • Additional numeric validation with proper bounds checking
  • Clear error messages with appropriate HTTP status codes
  • Handles edge cases like non-numeric or negative site IDs

35-51: Proper authentication and authorization implementation.

The security checks correctly implement:

  • Authentication verification using existing user session
  • Authorization check using the established getUserHasAccessToSite utility
  • Appropriate HTTP status codes (401 for auth, 403 for authorization)
  • Clear error messages for debugging

53-81: Solid database integration with defensive programming.

The database query and error handling demonstrate good practices:

  • Efficient single-field selection using Drizzle ORM
  • Proper use of eq condition and limit(1) for performance
  • 404 handling for non-existent sites
  • Defensive Array.isArray check for data consistency
  • Comprehensive error handling with logging and appropriate HTTP responses
server/src/api/sites/updateSiteExcludedIPs.ts (4)

10-13: LGTM!

Good use of Zod validation with appropriate constraints and string trimming.


38-54: LGTM!

Proper authentication and authorization checks with appropriate HTTP status codes.


56-71: LGTM!

Excellent error handling - collecting all validation errors before returning provides better user experience.


73-86: Move cache update outside the transaction

Updating the in-memory cache inside the database transaction can lead to inconsistency issues. If the cache update fails, the transaction rolls back correctly, but if the server crashes after DB commit but before cache update, they become out of sync.

-    // Update the database and cache atomically
-    await db.transaction(async (tx) => {
-      // Update the database
-      await tx
-        .update(sites)
-        .set({
-          excludedIPs: excludedIPs,
-          updatedAt: new Date().toISOString(),
-        })
-        .where(eq(sites.siteId, numericSiteId));
-
-      // Update the cache - if this fails, the transaction will rollback
-      siteConfig.updateSiteExcludedIPs(numericSiteId, excludedIPs);
-    });
+    // Update the database
+    await db
+      .update(sites)
+      .set({
+        excludedIPs: excludedIPs,
+        updatedAt: new Date().toISOString(),
+      })
+      .where(eq(sites.siteId, numericSiteId));
+
+    // Update the cache after successful DB update
+    try {
+      siteConfig.updateSiteExcludedIPs(numericSiteId, excludedIPs);
+    } catch (cacheError) {
+      // Log the error but don't fail the request since DB is already updated
+      console.error("Failed to update cache for site", numericSiteId, cacheError);
+      // Consider implementing a cache refresh mechanism here
+    }

Likely an incorrect or invalid review comment.

client/src/components/SiteSettings/IPExclusionManager.tsx (3)

57-90: LGTM!

Excellent validation logic with comprehensive error handling and user-friendly error messages.


92-99: LGTM!

Reset functionality correctly restores the original state with proper edge case handling.


105-183: LGTM!

Well-structured UI with excellent user experience design, including proper loading states, clear instructions, and intuitive controls.

client/src/lib/ipValidation.ts (3)

21-48: LGTM!

Well-structured validation function with clear pattern detection and appropriate delegation to specialized validators.


50-90: LGTM!

Correct implementation of IPv4/IPv6 validation with proper fallback logic and error handling.


92-130: LGTM!

Good implementation with clear documentation about IPv6 range limitations and helpful error messages.

client/src/api/admin/excludedIPs.ts (4)

1-21: LGTM!

Well-structured TypeScript interfaces with comprehensive type definitions.


23-41: LGTM!

Excellent error handling with graceful fallback for JSON parsing failures.


43-72: LGTM!

Proper POST request implementation with correct payload structure and error handling.


74-101: LGTM!

Excellent React Query implementation with proper cache invalidation and user feedback via toast notifications.

server/src/lib/ipUtils.ts (5)

8-56: LGTM!

Well-structured IP matching logic with proper pattern detection and delegation.


58-78: LGTM!

Correct implementation using the ip-address library's subnet checking capabilities.


88-103: LGTM!

Correct IPv4 range matching implementation with proper unsigned integer handling.


104-124: LGTM!

Appropriate handling of IPv6 ranges with clear documentation and helpful user guidance.


126-193: LGTM!

Validation implementation is consistent with the client-side version, ensuring uniform behavior across the stack.

Comment on lines +1 to +130
/**
* Client-side IP validation utility using ip-address library
* This matches the server-side validation logic in server/src/lib/ipUtils.ts
*/

import { Address4, Address6 } from "ip-address";

export interface IPValidationResult {
valid: boolean;
error?: string;
}

/**
* Validate if an IP pattern is valid
* Supports:
* - Single IPv4: 192.168.1.1
* - Single IPv6: 2001:db8::1, ::1, 2001::1
* - CIDR notation: 192.168.1.0/24, 2001:db8::/32
* - Range notation: 192.168.1.1-192.168.1.10 (IPv4 only, IPv6 ranges not supported)
*/
export function validateIPPattern(pattern: string): IPValidationResult {
try {
const trimmedPattern = pattern.trim();

if (!trimmedPattern) {
return { valid: true }; // Empty is valid (will be filtered out)
}

// Single IP
if (!trimmedPattern.includes("/") && !trimmedPattern.includes("-")) {
return validateSingleIP(trimmedPattern);
}

// CIDR notation
if (trimmedPattern.includes("/")) {
return validateCIDR(trimmedPattern);
}

// Range notation (IPv4 only)
if (trimmedPattern.includes("-")) {
return validateRange(trimmedPattern);
}

return { valid: false, error: "Unknown IP pattern format" };
} catch (error) {
return { valid: false, error: `Validation error: ${error}` };
}
}

/**
* Validate a single IP address (IPv4 or IPv6)
*/
function validateSingleIP(ip: string): IPValidationResult {
// Try IPv4 first
try {
new Address4(ip);
return { valid: true };
} catch {
// Try IPv6
try {
new Address6(ip);
return { valid: true };
} catch {
return { valid: false, error: "Invalid IP address format" };
}
}
}

/**
* Validate CIDR notation
*/
function validateCIDR(cidr: string): IPValidationResult {
try {
// Try IPv4 CIDR first
try {
new Address4(cidr);
return { valid: true };
} catch {
// Try IPv6 CIDR
try {
new Address6(cidr);
return { valid: true };
} catch {
return { valid: false, error: "Invalid CIDR notation" };
}
}
} catch (error) {
return { valid: false, error: `Error validating CIDR: ${error}` };
}
}

/**
* Validate range notation (IPv4 only)
*
* NOTE: Range notation is only supported for IPv4 addresses.
* IPv6 range notation is not supported due to the complexity of proper
* numerical comparison of 128-bit addresses. Use CIDR notation instead
* for IPv6 (e.g., 2001:db8::/32).
*/
function validateRange(range: string): IPValidationResult {
const parts = range.split("-").map(ip => ip.trim());
if (parts.length !== 2) {
return { valid: false, error: "Invalid range format" };
}

const [startIP, endIP] = parts;

if (!startIP || !endIP) {
return { valid: false, error: "Invalid range format" };
}

// Try IPv4 range
try {
new Address4(startIP);
new Address4(endIP);
return { valid: true };
} catch {
// Check if these are IPv6 addresses to provide a better error message
try {
new Address6(startIP);
new Address6(endIP);
return {
valid: false,
error: "IPv6 range notation not supported. Use CIDR notation instead (e.g., 2001:db8::/32)"
};
} catch {
return { valid: false, error: "Invalid IP addresses in range" };
}
}
} No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify client-server validation consistency

Since this mirrors server-side validation, ensure both implementations remain synchronized to avoid validation discrepancies.


🏁 Script executed:

#!/bin/bash
# Compare client and server IP validation implementations

echo "=== Comparing validation function signatures ==="
echo "Client validateIPPattern:"
rg -A 5 "export function validateIPPattern" client/src/lib/ipValidation.ts

echo -e "\nServer validateIPPattern:"
rg -A 5 "export function validateIPPattern" server/src/lib/ipUtils.ts

echo -e "\n=== Checking for validation logic differences ==="
# Look for any TODO or FIXME comments that might indicate inconsistencies
rg -i "todo|fixme" client/src/lib/ipValidation.ts server/src/lib/ipUtils.ts

echo -e "\n=== Comparing error messages ==="
echo "Client error messages:"
rg "error:" client/src/lib/ipValidation.ts | grep -o '"[^"]*"' | sort | uniq

echo -e "\nServer error messages:"
rg "error:" server/src/lib/ipUtils.ts | grep -o '"[^"]*"' | sort | uniq

Length of output: 1825


Align empty‐string handling in client and server IP validation

The client’s validateIPPattern treats an empty or all-whitespace pattern as valid, but the server implementation currently does not, leading to inconsistent behavior when users submit blank input. Please add the same check in server/src/lib/ipUtils.ts so both sides remain in sync.

• In server/src/lib/ipUtils.ts, immediately after trimming the input (around line 3 of validateIPPattern), insert:

 export function validateIPPattern(pattern: string): { valid: boolean; error?: string } {
   try {
     const trimmedPattern = pattern.trim();
+    if (!trimmedPattern) {
+      return { valid: true }; // Empty pattern is valid (will be filtered out)
+    }
 
     // Single IP
     if (!trimmedPattern.includes("/") && !trimmedPattern.includes("-")) {

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In server/src/lib/ipUtils.ts around line 3 inside the validateIPPattern
function, add a check after trimming the input string to return valid true if
the input is empty or all whitespace, matching the client-side behavior. This
ensures consistent handling of empty or blank IP patterns on both client and
server sides.

This commit updates the logging mechanism in various modules to utilize a centralized logger instead of console statements. Changes include:

- Replaced `console.log` and `console.warn` with `logger.info` and `logger.warn` for consistent logging practices.
- Enhanced error handling by logging errors with context, improving traceability.
- Updated the geolocation and session replay services to log relevant events and errors using the new logger.

These improvements aim to standardize logging across the application, facilitating better monitoring and debugging.
This commit refines the IP exclusion functionality by moving the `isIPExcluded` method to the `siteConfig` class, improving code organization and reusability. Additionally, it updates the uptime monitoring service initialization to ensure it runs correctly in the background, enhancing the application's reliability.

Changes include:
- Moved IP exclusion check to `siteConfig` for better encapsulation.
- Updated the uptime monitoring service to remove commented-out code and ensure proper logging on initialization success or failure.
This commit updates the IP exclusion management logic to utilize the `authedFetch` utility for API calls, enhancing code consistency and security. The changes include:

- Replaced direct fetch calls with `authedFetch` in both `fetchExcludedIPs` and `updateExcludedIPs` functions.
- Improved error handling and response parsing for better reliability.
- Minor formatting adjustments in the IP exclusion manager component for improved readability.

These updates aim to streamline the API interaction process and ensure proper authentication for IP exclusion operations.
…arity

This commit updates the IP exclusion functionality by modifying the `isIPExcluded` method to directly retrieve excluded IPs based on the site ID, enhancing code organization. Additionally, it adjusts the IP exclusion checks in the session replay and event tracking services to utilize the updated method signature.

Changes include:
- Refactored `isIPExcluded` to accept site ID and fetch excluded IPs internally.
- Updated session replay and event tracking logic to align with the new method signature.
- Improved client-side IP validation feedback in the IP exclusion manager component.
@coderabbitai coderabbitai bot mentioned this pull request Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants