Add per-website IP blocklist to exclude IPs from statistics#4378
Conversation
Add a new field blockedIps to the Website model to allow excluding specific IP addresses from statistics on a per-website basis. - Database: Add blocked_ips VARCHAR(500) column to website table - Backend: Check per-website blocked IPs in /api/send and /api/record endpoints - API: Add blockedIps to website update schema - Frontend: Add WebsiteBlocklist component in website settings - i18n: Add English locale strings for the feature - Tests: Add tests for hasBlockedIp with extra IPs parameter
|
@mchev is attempting to deploy a commit to the Umami Software Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR adds per-website IP blocking to umami, allowing site owners to exclude specific IPs and CIDR ranges from their analytics without relying solely on the global
Confidence Score: 3/5Two defects in the changed code should be fixed before merging: one prevents users from ever clearing their blocklist, and the other removes a deliberate cache optimization on every tracking request. The update route has a data-loss bug where a user who clears the textarea and saves will silently keep their old blocklist forever with no error. Separately, the tracking route now unconditionally calls fetchWebsite on every POST, bypassing the cache guard that was specifically there to skip the lookup for cached sessions. Both issues are on actively-used code paths. src/app/api/websites/[websiteId]/route.ts (blockedIps clear bug) and src/app/api/send/route.ts (unconditional fetchWebsite call). Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant C as Client
participant S as POST /api/send
participant DB as fetchWebsite (Redis/DB)
participant D as hasBlockedIp()
C->>S: "POST {websiteId, cache-token, ...}"
S->>DB: fetchWebsite(websiteId) [always, even if cache valid]
DB-->>S: website (incl. blockedIps)
S->>D: hasBlockedIp(ip) [global IGNORE_IP]
D-->>S: false
S->>D: hasBlockedIp(ip, website.blockedIps) [per-website]
alt IP is blocked
D-->>S: true
S-->>C: "{beep: boop}"
else IP is allowed
D-->>S: false
S->>S: createSession / saveEvent
S-->>C: "{cache, sessionId, visitId}"
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant C as Client
participant S as POST /api/send
participant DB as fetchWebsite (Redis/DB)
participant D as hasBlockedIp()
C->>S: "POST {websiteId, cache-token, ...}"
S->>DB: fetchWebsite(websiteId) [always, even if cache valid]
DB-->>S: website (incl. blockedIps)
S->>D: hasBlockedIp(ip) [global IGNORE_IP]
D-->>S: false
S->>D: hasBlockedIp(ip, website.blockedIps) [per-website]
alt IP is blocked
D-->>S: true
S-->>C: "{beep: boop}"
else IP is allowed
D-->>S: false
S->>S: createSession / saveEvent
S-->>C: "{cache, sessionId, visitId}"
end
Reviews (1): Last reviewed commit: "Add per-website IP blocklist feature" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Find website | ||
| if (!cache?.websiteId) { | ||
| const website = await fetchWebsite(websiteId); | ||
| website = await fetchWebsite(websiteId); | ||
|
|
||
| if (!website) { | ||
| return badRequest({ message: 'Website not found.' }); | ||
| } | ||
| if (!website) { | ||
| return badRequest({ message: 'Website not found.' }); | ||
| } |
There was a problem hiding this comment.
fetchWebsite behind !cache?.websiteId, so repeat hits from cached sessions (the vast majority of tracking traffic) never triggered a lookup. The new code unconditionally calls fetchWebsite on every POST, even when the cache token is fully valid. On a high-traffic site this adds a Redis or DB round-trip to every single tracking event. If fetchWebsite is already Redis-cached the overhead per request is small, but the cumulative cost at scale is real and the deliberate cache skip is gone.
| test('hasBlockedIp: combines IGNORE_IP and extraIps correctly', () => { | ||
| process.env.IGNORE_IP = '10.0.0.0/8'; | ||
|
|
||
| expect(hasBlockedIp('10.0.0.5', '192.168.1.0/24')).toBe(true); | ||
| }); |
There was a problem hiding this comment.
process.env.IGNORE_IP is set at the end of the file but never cleaned up. The beforeEach hook deletes it before every test in this file, but if any other test module runs in the same worker process after this one, it will inherit the dirty env state. Add a reset inside this test's finally block.
| test('hasBlockedIp: combines IGNORE_IP and extraIps correctly', () => { | |
| process.env.IGNORE_IP = '10.0.0.0/8'; | |
| expect(hasBlockedIp('10.0.0.5', '192.168.1.0/24')).toBe(true); | |
| }); | |
| test('hasBlockedIp: combines IGNORE_IP and extraIps correctly', () => { | |
| process.env.IGNORE_IP = '10.0.0.0/8'; | |
| try { | |
| expect(hasBlockedIp('10.0.0.5', '192.168.1.0/24')).toBe(true); | |
| } finally { | |
| delete process.env.IGNORE_IP; | |
| } | |
| }); |
…axLength
- Fix blockedIps null coalescing: blockedIps ?? undefined → blockedIps !== undefined ? blockedIps : undefined
so that explicitly passing null actually clears the field in Prisma
- Restore cache guard on fetchWebsite in /api/send to avoid
unnecessary Redis/DB round-trips on cached sessions
- Clean up process.env.IGNORE_IP in test with try/finally
- Add maxLength={500} to blocked IPs textarea for early UX feedback
mchev
force-pushed
the
feature/per-website-ip-blocklist
branch
from
8c4e5fa to
2e93ae7
Compare
1 participant
Summary
Adds the ability to configure IP addresses (with CIDR support) to exclude from statistics on a per-website basis. This is useful for developers who want to exclude their own traffic from analytics without relying on the global
IGNORE_IPenvironment variable.Changes
Database
blocked_ips VARCHAR(500)column to thewebsitetable (via migrationprisma/migrations/21_add_blocked_ips/)blockedIpsfield on theWebsitemodelBackend
hasBlockedIp()insrc/lib/detect.tsto accept an optionalextraIpsparameter, merging it with the globalIGNORE_IPenv varPOST /api/send(main tracking endpoint) andPOST /api/record(session replay/heatmap recording){ beep: "boop" }) — same pattern as the bot check, so blocked visitors are not awareAPI
blockedIpsfield to the website update schema (POST /api/websites/[id])Frontend
i18n
blocked-ipslabel andblocked-ips-descriptionmessage inen-USanden-GBlocalesTests
hasBlockedIp()covering:extraIpsextraIpsIGNORE_IP+extraIpsUsage
192.168.1.0/24)Visitors whose IP matches any entry will be silently excluded from analytics for that website.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.