Skip to content

feat: migrate analytics from ClickHouse to PostHog#62

Open
mrmps wants to merge 1 commit intomainfrom
migrate-posthog
Open

feat: migrate analytics from ClickHouse to PostHog#62
mrmps wants to merge 1 commit intomainfrom
migrate-posthog

Conversation

@mrmps
Copy link
Owner

@mrmps mrmps commented Feb 5, 2026

Summary

  • Replace self-hosted ClickHouse with PostHog (posthog-js + posthog-node SDKs)
  • All analytics events (request_event, ad_event) now flow through PostHog's capture API
  • Admin dashboard queries converted to HogQL (PostHog's ClickHouse-compatible SQL via properties.*)
  • Deleted all ClickHouse infrastructure (Docker, schema, scripts, client library)

Changes

  • New: lib/posthog.ts — server-side PostHog client (trackEvent, trackAdEvent, queryPostHog, getBufferStats, closePostHog)
  • New: components/providers/posthog-provider.tsx — client-side pageview + page leave tracking
  • Updated: app/layout.tsx — wraps children with <PostHogProvider>
  • Updated: server/routes/admin.ts — all 37 dashboard queries converted to HogQL
  • Updated: lib/alerting.ts — error rate queries converted to HogQL
  • Updated: Import paths in request-context.ts, memory-monitor.ts, gravity.ts
  • Updated: Env configs (server/env.ts, lib/env.ts, .env.example, docker-compose.yml, railway.template.json)
  • Deleted: lib/clickhouse.ts, docker/clickhouse/, docs/clickhouse-schema.sql, scripts/analyze-sources.ts, scripts/source-analysis-queries.sql

Environment Variables

Old (remove) New (add)
CLICKHOUSE_URL POSTHOG_API_KEY
CLICKHOUSE_USER POSTHOG_HOST
CLICKHOUSE_PASSWORD POSTHOG_PROJECT_ID
CLICKHOUSE_DATABASE POSTHOG_PERSONAL_API_KEY
NEXT_PUBLIC_POSTHOG_KEY
NEXT_PUBLIC_POSTHOG_HOST

Test plan

  • bun run typecheck — zero TS errors
  • bun test — 203 tests pass, 0 failures
  • grep -ri clickhouse — no remaining imports or code references
  • Deploy to staging and verify events appear in PostHog
  • Verify admin dashboard loads with HogQL queries
  • Verify client-side pageview tracking in PostHog

🤖 Generated with Claude Code


Open with Devin

Summary by CodeRabbit

Release Notes

  • New Features

    • PostHog analytics integration now fully enabled throughout the application, providing real-time event tracking and performance insights.
  • Chores

    • Analytics infrastructure updated; deployment and local development environment simplified for improved setup efficiency.

Replace self-hosted ClickHouse with PostHog (posthog-js + posthog-node).
All analytics events (request_event, ad_event) now flow through PostHog's
capture API; dashboard queries use HogQL against PostHog's events table.

- Add lib/posthog.ts (server-side SDK: trackEvent, trackAdEvent, queryPostHog)
- Add components/providers/posthog-provider.tsx (client-side pageview tracking)
- Convert all admin dashboard SQL to HogQL (properties.* column access)
- Convert alerting queries to HogQL
- Remove lib/clickhouse.ts, docker/clickhouse/, docs/clickhouse-schema.sql
- Remove scripts/analyze-sources.ts and source-analysis-queries.sql
- Update env configs (POSTHOG_* replaces CLICKHOUSE_*)
- Update docker-compose.yml, railway.template.json, package.json

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Feb 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
smry Ready Ready Preview, Comment Feb 5, 2026 8:08pm

Request Review

@railway-app
Copy link

railway-app bot commented Feb 5, 2026

🚅 Deployed to the SMRY-pr-62 environment in smry

Service Status Web Updated (UTC)
smry-api ✅ Success (View Logs) Feb 5, 2026 at 8:09 pm
SMRY ✅ Success (View Logs) Web Feb 5, 2026 at 8:09 pm

@coderabbitai
Copy link

coderabbitai bot commented Feb 5, 2026

📝 Walkthrough

Walkthrough

This PR migrates the application's analytics infrastructure from ClickHouse to PostHog. It removes ClickHouse services, dependencies, and schema documentation; introduces new PostHog client integration with server and client-side providers; updates all analytics queries from ClickHouse SQL to HogQL; and refactors event tracking and alerting modules to use PostHog APIs.

Changes

Cohort / File(s) Summary
Environment & Configuration
.env.example, lib/env.ts, server/env.ts, railway.template.json, docker-compose.yml
Replaced ClickHouse environment variables (CLICKHOUSE_URL, CLICKHOUSE_USER, CLICKHOUSE_PASSWORD, CLICKHOUSE_DATABASE) with PostHog equivalents (POSTHOG_API_KEY, POSTHOG_HOST, POSTHOG_PROJECT_ID, POSTHOG_PERSONAL_API_KEY) across configuration files and Docker setup.
ClickHouse Removal
lib/clickhouse.ts, docker/clickhouse/Dockerfile, docker/clickhouse/memory.xml, docs/clickhouse-schema.sql
Completely removed ClickHouse client module (767 lines), Docker configuration, memory optimization settings, and comprehensive SQL schema documentation.
PostHog Integration
lib/posthog.ts, components/providers/posthog-provider.tsx, app/layout.tsx
Added new PostHog client module (239 lines) with event tracking and HogQL query support; introduced PostHogProvider component for client-side initialization and automatic pageview capture; integrated provider into root layout.
Analytics Logic Migration
lib/alerting.ts, lib/memory-monitor.ts, lib/request-context.ts, server/routes/admin.ts
Migrated all analytics queries and event tracking from ClickHouse to PostHog; refactored admin route queries (426 lines added) from ClickHouse SQL to HogQL with properties.* field mappings; updated alerting and memory monitoring to use PostHog data source.
Ad Tracking Migration
server/routes/gravity.ts
Switched ad impression logging from ClickHouse to PostHog client.
Scripts & Analysis Removal
scripts/analyze-sources.ts, scripts/source-analysis-queries.sql
Removed source effectiveness analysis TypeScript script (380 lines) and comprehensive SQL analysis queries (282 lines) that were ClickHouse-dependent.
Package & Dependencies
package.json
Removed @clickhouse/client dependency; added posthog-js and posthog-node; updated dev script to remove docker-compose startup.
Tests
server/index.test.ts
Expanded allowed HTTP response status codes from [200, 500] to [200, 401, 500] across analytics and admin endpoint tests to accommodate missing authentication scenarios.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Browser as Browser/App
    participant Layout as app/layout.tsx
    participant PostHogProvider as PostHogProvider
    participant PostHogJS as PostHog JS SDK
    participant PostHogAPI as PostHog API

    User->>Browser: Navigate to app
    Browser->>Layout: Load layout
    Layout->>PostHogProvider: Wrap with provider
    PostHogProvider->>PostHogJS: Initialize (NEXT_PUBLIC_POSTHOG_KEY)
    PostHogJS->>PostHogAPI: Connect
    
    User->>Browser: Navigate to route
    Browser->>PostHogProvider: Route change detected
    PostHogProvider->>PostHogProvider: usePathname/useSearchParams
    PostHogProvider->>PostHogJS: Construct URL + capture($pageview)
    PostHogJS->>PostHogAPI: Send pageview event
Loading
sequenceDiagram
    participant Server as Server Code
    participant PostHogClient as PostHog Client (posthog.ts)
    participant PostHogSDK as PostHog Node SDK
    participant PostHogAPI as PostHog API

    Server->>PostHogClient: trackEvent(AnalyticsEvent)
    PostHogClient->>PostHogClient: getClient() lazy init
    PostHogClient->>PostHogSDK: capture(request_event)
    PostHogSDK->>PostHogAPI: Flush to API
    
    Server->>PostHogClient: queryPostHog(HogQL)
    PostHogClient->>PostHogAPI: POST /api/query (HogQL)
    PostHogAPI->>PostHogClient: Query results
    PostHogClient->>Server: Typed results []
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • PR #53: Modifies server/routes/admin.ts analytics queries and result shapes; this PR refactors the same file from ClickHouse to PostHog HogQL.
  • PR #47: Changes ad-tracking interfaces, ad-event types, and analytics client; directly related as this PR migrates ad tracking from ClickHouse to PostHog.
  • PR #57: Updates server/routes/gravity.ts ad tracking and ad-event interfaces; related since this PR also refactors the same gravity tracking route to use PostHog.

Poem

🐰 ClickHouse is gone, PostHog is here,
Analytics dancing, crystal clear!
From SQL to HogQL we leap,
Events captured, data deep,
Tracking pageviews, events in flight,
Every hop logs data right! 📊✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 64.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: migrate analytics from ClickHouse to PostHog' directly and clearly describes the primary change: a complete migration of the analytics backend from ClickHouse to PostHog. The changeset confirms this is the main focus with removal of ClickHouse infrastructure, addition of PostHog libraries, and conversion of queries across the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch migrate-posthog

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

Comment @coderabbitai help to get the list of available commands and usage tips.

escapeForClickhouse(str).replace(/%/g, "\\%").replace(/_/g, "\\_");
const escapeStr = (str: string) => str.replace(/\\/g, "\\\\").replace(/'/g, "''");
const escapeForLike = (str: string) =>
escapeStr(str).replace(/%/g, "\\%").replace(/_/g, "\\_");

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This does not escape backslash characters in the input.

Copilot Autofix

AI 4 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

escapeForClickhouse(str).replace(/%/g, "\\%").replace(/_/g, "\\_");
const escapeStr = (str: string) => str.replace(/\\/g, "\\\\").replace(/'/g, "''");
const escapeForLike = (str: string) =>
escapeStr(str).replace(/%/g, "\\%").replace(/_/g, "\\_");

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This does not escape backslash characters in the input.

Copilot Autofix

AI 4 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

@paragon-review
Copy link

paragon-review bot commented Feb 5, 2026

Paragon Summary

This pull request review identified 8 issues across 4 categories in 22 files. The review analyzed code changes, potential bugs, security vulnerabilities, performance issues, and code quality concerns using automated analysis tools.

This PR migrates the analytics system from self-hosted ClickHouse to PostHog, replacing infrastructure and converting all dashboard queries to HogQL while updating environment variables for the new SDKs.

Key changes:

  • Migrated analytics from ClickHouse to PostHog SDKs
  • Added PostHog client (lib/posthog.ts) and provider (components/providers/posthog-provider.tsx)
  • Converted 37 dashboard queries and alerting to HogQL
  • Removed all ClickHouse infrastructure and related code
  • Updated env vars (removed CLICKHOUSE_*, added POSTHOG_*)

Confidence score: 2/5

  • This PR has high risk due to 1 critical issue that require immediate attention before merge
  • Score reflects critical security vulnerabilities, data loss risks, or system stability issues
  • Pay close attention to critical findings and address them before proceeding

22 files reviewed, 8 comments

Severity breakdown: Critical: 1, High: 2, Medium: 2, Low: 3

Dashboard


// Build WHERE clause for filtered queries
// Build WHERE clause for filtered queries (HogQL – properties.* prefix)
const buildWhereClause = (options: {
Copy link

Choose a reason for hiding this comment

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

Security: HogQL queries vulnerable to SQL injection via numeric params

HogQL queries vulnerable to SQL injection via numeric params. Attackers could exfiltrate analytics data. Validate and bound hours/minutes before interpolation.

View Details

Location: server/routes/admin.ts (lines 424)

Analysis

HogQL queries vulnerable to SQL injection via numeric params

What fails The hours and minutes variables are interpolated directly into HogQL queries via string concatenation without parameterization or bounds checking
Result Malicious HogQL queries could potentially read/modify PostHog data or cause DoS
Expected Numeric values should be validated and bounded before SQL interpolation
Impact Critical security vulnerability. HogQL injection could expose all analytics data stored in PostHog or cause service disruption.
How to reproduce
GET /api/admin?range=100;DROP TABLE events--
Patch Details
-      conditions.push(`timestamp > now() - INTERVAL ${timeInterval}`);
+      const safeHours = Math.min(Math.max(1, Math.floor(hours)), 168); // cap at 7 days
+      conditions.push(`timestamp > now() - INTERVAL ${safeHours} HOUR`);
AI Fix Prompt
Fix this issue: HogQL queries vulnerable to SQL injection via numeric params. Attackers could exfiltrate analytics data. Validate and bound hours/minutes before interpolation.

Location: server/routes/admin.ts (lines 424)
Problem: The hours and minutes variables are interpolated directly into HogQL queries via string concatenation without parameterization or bounds checking
Current behavior: Malicious HogQL queries could potentially read/modify PostHog data or cause DoS
Expected: Numeric values should be validated and bounded before SQL interpolation
Steps to reproduce: GET /api/admin?range=100;DROP TABLE events--

Provide a code fix.


Tip: Reply with @paragon-run to automatically fix this issue

if (hostnameFilter) {
conditions.push(`hostname = '${escapeForClickhouse(hostnameFilter)}'`);
conditions.push(`properties.hostname = '${escapeStr(hostnameFilter)}'`);
}
Copy link

Choose a reason for hiding this comment

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

Security: Ad device filter allows SQL injection via unsanitized string

Ad device filter allows SQL injection via unsanitized string. Attackers can exfiltrate data. Apply escapeStr() to adDeviceFilter parameter.

View Details

Location: server/routes/admin.ts (lines 446)

Analysis

Ad device filter allows SQL injection via unsanitized string. Attackers can exfiltrate data

What fails The adDeviceFilter parameter is directly interpolated into HogQL queries without sanitization. The escapeStr function exists but isn't applied to adDeviceFilter in ad analytics queries.
Result SQL injection vulnerability allowing data exfiltration or query manipulation
Expected All user input should be escaped before SQL interpolation
Impact Critical security issue. Attacker could access all analytics data, inject malicious queries, or cause denial of service.
How to reproduce
GET /api/admin?adDevice=mobile' OR 1=1--
Patch Details
-            ${adDeviceFilter ? `AND properties.device_type = '${adDeviceFilter}'` : ''}
+            ${adDeviceFilter ? `AND properties.device_type = '${adDeviceFilter.replace(/'/g, "''")}'` : ''}
AI Fix Prompt
Fix this issue: Ad device `filter` allows SQL injection via unsanitized string. Attackers can exfiltrate data. Apply escapeStr() to adDeviceFilter parameter.

Location: server/routes/admin.ts (lines 446)
Problem: The adDeviceFilter parameter is directly interpolated into HogQL queries without sanitization. The escapeStr function exists but isn't applied to adDeviceFilter in ad analytics queries.
Current behavior: SQL injection vulnerability allowing data exfiltration or query manipulation
Expected: All user input should be escaped before SQL interpolation
Steps to reproduce: GET /api/admin?adDevice=mobile' OR 1=1--

Provide a code fix.


Tip: Reply with @paragon-run to automatically fix this issue

const search = searchParams?.toString();
if (search) url += `?${search}`;
ph.capture("$pageview", { $current_url: url });
}
Copy link

Choose a reason for hiding this comment

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

Bug: PostHog SDK race condition on initialization

PostHog SDK race condition on initialization. Events may be silently dropped on first render. Check posthog.__loaded before passing to PHProvider.

View Details

Location: components/providers/posthog-provider.tsx (lines 19)

Analysis

PostHog SDK race condition on initialization. Events may be silently dropped on first render

What fails The PHProvider is passed posthog (the uninitialized singleton) immediately, while posthog.init() runs in a useEffect. This creates a race condition where PostHogPageView and children may attempt to capture events before initialization completes.
Result Events captured during first render may be lost or cause 'PostHog not initialized' warnings
Expected All events should be queued until initialization completes, or initialization should happen synchronously before provider renders children
Impact Loss of analytics data for initial page loads, affecting traffic metrics accuracy.
How to reproduce
1. Load page with cold cache
2. Check browser console for PostHog errors
3. Observe pageview events may be silently dropped on initial render
AI Fix Prompt
Fix this issue: PostHog SDK race condition on initialization. Events may be silently dropped on first render. Check posthog.__loaded before passing to PHProvider.

Location: components/providers/posthog-provider.tsx (lines 19)
Problem: The PHProvider is passed posthog (the uninitialized singleton) immediately, while posthog.init() runs in a useEffect. This creates a race condition where PostHogPageView and children may attempt to capture events before initialization completes.
Current behavior: Events captured during first render may be lost or cause 'PostHog not initialized' warnings
Expected: All events should be queued until initialization completes, or initialization should happen synchronously before provider renders children
Steps to reproduce: 1. Load page with cold cache
2. Check browser console for PostHog errors
3. Observe pageview events may be silently dropped on initial render

Provide a code fix.


Tip: Reply with @paragon-run to automatically fix this issue

const host = process.env.NEXT_PUBLIC_POSTHOG_HOST;
if (!key || !host) return;

posthog.init(key, {
Copy link

Choose a reason for hiding this comment

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

Bug: Missing PostHog defaults config causes SSR hydration errors

Missing PostHog defaults config causes SSR hydration errors. React hydration mismatches may occur. Add defaults: '2025-11-30' to posthog.init().

View Details

Location: components/providers/posthog-provider.tsx (lines 31)

Analysis

Missing PostHog defaults config causes SSR hydration errors. React hydration mismatches may occur

What fails PostHog official Next.js documentation recommends setting defaults: '2025-11-30' to avoid SSR hydration errors. Without this, PostHog may inject scripts causing React hydration mismatches.
Result React hydration warnings/errors in console, potential UI glitches
Expected No hydration errors when using PostHog with Next.js SSR
Impact Poor developer experience, potential user-facing issues from hydration mismatches.
How to reproduce
1. Load page server-rendered
2. Watch for 'Prop dangerouslySetInnerHTML did not match' warnings
3. Especially with features like surveys enabled
Patch Details
-    posthog.init(key, {
-      api_host: host,
-      capture_pageview: false, // we handle it manually above
-      capture_pageleave: true,
-    });
+    posthog.init(key, {
+      api_host: host,
+      capture_pageview: false,
+      capture_pageleave: true,
+      defaults: '2025-11-30',
+    });
AI Fix Prompt
Fix this issue: Missing PostHog defaults config causes SSR hydration errors. React hydration mismatches may occur. Add defaults: '2025-11-30' to posthog.init().

Location: components/providers/posthog-provider.tsx (lines 31)
Problem: PostHog official Next.js documentation recommends setting defaults: '2025-11-30' to avoid SSR hydration errors. Without this, PostHog may inject scripts causing React hydration mismatches.
Current behavior: React hydration warnings/errors in console, potential UI glitches
Expected: No hydration errors when using PostHog with Next.js SSR
Steps to reproduce: 1. Load page server-rendered
2. Watch for 'Prop dangerouslySetInnerHTML did not match' warnings
3. Especially with features like surveys enabled

Provide a code fix.


Tip: Reply with @paragon-run to automatically fix this issue

// trackEvent – captures request analytics
// ---------------------------------------------------------------------------

export function trackEvent(event: Partial<AnalyticsEvent>): void {
Copy link

Choose a reason for hiding this comment

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

Bug: Server PostHog may drop events before SDK initialization

Server PostHog may drop events before SDK initialization. Events during startup could be lost. Add initialization check or explicit buffering.

View Details

Location: lib/posthog.ts (lines 119)

Analysis

Server PostHog may drop events before SDK initialization. Events during startup could be lost

What fails The server PostHog client (posthog-node) captures events synchronously but the SDK may not be fully initialized. The deleted ClickHouse code had explicit buffer management; this relies entirely on PostHog SDK internals.
Result Events captured before SDK handshake completes may be lost
Expected Events should be reliably captured regardless of timing
Impact Potential data loss during server startup or first requests.
How to reproduce
1. Start server with valid env vars
2. Immediately make API requests before SDK finishes initialization
3. Events may be dropped
AI Fix Prompt
Fix this issue: Server PostHog may drop events before SDK initialization. Events during startup could be lost. Add initialization check or explicit buffering.

Location: lib/posthog.ts (lines 119)
Problem: The server PostHog client (posthog-node) captures events synchronously but the SDK may not be fully initialized. The deleted ClickHouse code had explicit buffer management; this relies entirely on PostHog SDK internals.
Current behavior: Events captured before SDK handshake completes may be lost
Expected: Events should be reliably captured regardless of timing
Steps to reproduce: 1. Start server with valid env vars
2. Immediately make API requests before SDK finishes initialization
3. Events may be dropped

Provide a code fix.


Tip: Reply with @paragon-run to automatically fix this issue

@@ -38,15 +38,16 @@ interface TopError {
async function getErrorRateStats(): Promise<ErrorRateStats | null> {
Copy link

Choose a reason for hiding this comment

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

Bug: Alerting silently fails when PostHog env vars missing

Alerting silently fails when PostHog env vars missing. Team won't know monitoring is disabled. Log explicit warning when queryPostHog returns empty.

View Details

Location: lib/alerting.ts (lines 38)

Analysis

Alerting silently fails when PostHog env vars missing. Team won't know monitoring is disabled

What fails The alerting system depends on queryPostHog which silently returns empty arrays when PostHog env vars are missing. Unlike the old ClickHouse implementation which logged warnings, this fails silently.
Result Alerting silently disabled, team unaware they have no error monitoring
Expected Log clear warning when PostHog is not configured, or fail startup if alerting is required
Impact Error rate spikes won't trigger alerts if PostHog is misconfigured. Could lead to extended outages going unnoticed.
How to reproduce
1. Deploy without POSTHOG_PERSONAL_API_KEY
2. Alerting cron runs
3. Logs show 'No data from PostHog' but no indication env vars are missing
AI Fix Prompt
Fix this issue: Alerting silently fails when PostHog env vars missing. Team won't know monitoring is disabled. Log explicit warning when queryPostHog returns empty.

Location: lib/alerting.ts (lines 38)
Problem: The alerting system depends on queryPostHog which silently returns empty arrays when PostHog env vars are missing. Unlike the old ClickHouse implementation which logged warnings, this fails silently.
Current behavior: Alerting silently disabled, team unaware they have no error monitoring
Expected: Log clear warning when PostHog is not configured, or fail startup if alerting is required
Steps to reproduce: 1. Deploy without POSTHOG_PERSONAL_API_KEY
2. Alerting cron runs
3. Logs show 'No data from PostHog' but no indication env vars are missing

Provide a code fix.


Tip: Reply with @paragon-run to automatically fix this issue

}
}

// Register shutdown handler
Copy link

Choose a reason for hiding this comment

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

Bug: Async shutdown handler won't flush buffered events

Async shutdown handler won't flush buffered events. Analytics may be lost on process exit. Use SIGTERM handler with proper async waiting.

View Details

Location: lib/posthog.ts (lines 232)

Analysis

Async shutdown handler won't flush buffered events. Analytics may be lost on process exit

What fails The beforeExit event listener has an async callback, but Node.js beforeExit doesn't wait for async operations. The SDK's pending events may not be flushed before the process terminates.
Result Analytics events in the PostHog SDK buffer may be lost on server shutdown
Expected Use appropriate shutdown handling that waits for async operations
Impact Could lose final batch of analytics on graceful shutdown.
How to reproduce
1. Server receives events
2. Process exits immediately (SIGTERM)
3. Buffered events are lost
AI Fix Prompt
Fix this issue: Async shutdown handler won't flush buffered events. Analytics may be lost on process exit. Use SIGTERM handler with proper `async` waiting.

Location: lib/posthog.ts (lines 232)
Problem: The beforeExit event listener has an async callback, but Node.js beforeExit doesn't wait for async operations. The SDK's pending events may not be flushed before the process terminates.
Current behavior: Analytics events in the PostHog SDK buffer may be lost on server shutdown
Expected: Use appropriate shutdown handling that waits for async operations
Steps to reproduce: 1. Server receives events
2. Process exits immediately (SIGTERM)
3. Buffered events are lost

Provide a code fix.


Tip: Reply with @paragon-run to automatically fix this issue

// trackEvent – captures request analytics
// ---------------------------------------------------------------------------

export function trackEvent(event: Partial<AnalyticsEvent>): void {
Copy link

Choose a reason for hiding this comment

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

Bug: Client IP tracking removed without replacement

Client IP tracking removed without replacement. Geographic analytics lost. Pass client IP explicitly to PostHog server-side events.

View Details

Location: lib/posthog.ts (lines 119)

Analysis

Client IP tracking removed without replacement. Geographic analytics lost

What fails The AnalyticsEvent interface includes client_ip field, but tracking happens server-side via posthog-node which doesn't automatically capture client IP. The caller must explicitly pass it.
Result Loss of geographic/IP analytics that were available with ClickHouse
Expected Server-side events should pass client IP to PostHog for geo analytics
Impact Loss of valuable analytics data for geographic analysis and fraud detection.
How to reproduce
1. Check PostHog dashboard for request_events
2. Observe client IP data is likely empty or server IP
AI Fix Prompt
Fix this issue: Client IP tracking removed without replacement. Geographic analytics lost. Pass client IP explicitly to PostHog server-side events.

Location: lib/posthog.ts (lines 119)
Problem: The AnalyticsEvent interface includes client_ip field, but tracking happens server-side via posthog-node which doesn't automatically capture client IP. The caller must explicitly pass it.
Current behavior: Loss of geographic/IP analytics that were available with ClickHouse
Expected: Server-side events should pass client IP to PostHog for geo analytics
Steps to reproduce: 1. Check PostHog dashboard for request_events
2. Observe client IP data is likely empty or server IP

Provide a code fix.


Tip: Reply with @paragon-run to automatically fix this issue

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 5, 2026

Greptile Overview

Greptile Summary

This PR successfully migrates analytics infrastructure from self-hosted ClickHouse to PostHog's managed service. The migration removes significant operational complexity (Docker containers, schema management, custom buffering) in favor of PostHog's SDK-managed infrastructure.

Key changes:

  • Replaced @clickhouse/client with posthog-js (client) and posthog-node (server)
  • Converted 37+ dashboard queries from ClickHouse SQL to HogQL (PostHog's ClickHouse-compatible query language)
  • All event properties now prefixed with properties.* per PostHog's schema
  • Removed 1,400+ lines: ClickHouse Docker setup, schema definitions, custom analytics scripts
  • Client-side pageview tracking added via PostHogProvider
  • Environment variables changed from required to optional for graceful degradation

Query conversion notes:

  • HogQL queries use FROM events WHERE event = 'request_event' instead of FROM request_events
  • Numeric fields cast with toFloat64() for aggregations (e.g., avg(toFloat64(properties.duration_ms)))
  • All custom properties accessed via properties. prefix

Tests pass (203/203), typechecks clean. The PostHog client initialization in the provider component reinitializes on every render, though PostHog's SDK likely handles this idempotently.

Confidence Score: 4/5

  • Safe to merge with minor PostHog SDK initialization concern that likely self-resolves
  • Strong migration with comprehensive query conversions and proper type casting. All tests pass. Minor concern: PostHog client may reinitialize on every render in the provider component, though SDK likely handles this. Admin queries properly converted to HogQL with event filtering. Infrastructure cleanly removed.
  • Check components/providers/posthog-provider.tsx - PostHog initialization pattern may cause re-init on every render

Important Files Changed

Filename Overview
lib/posthog.ts New PostHog client implementation with event tracking, HogQL querying, and graceful shutdown
components/providers/posthog-provider.tsx Client-side PostHog provider with manual pageview tracking and pageleave capture
server/routes/admin.ts Converted 37 dashboard queries from ClickHouse SQL to HogQL with properties.* prefix
lib/alerting.ts Updated error rate monitoring queries to use HogQL and PostHog query API
server/env.ts Replaced required ClickHouse env vars with optional PostHog configuration
lib/env.ts Added optional PostHog env vars for both server and client-side tracking
package.json Removed @clickhouse/client, added posthog-js and posthog-node, removed ClickHouse from dev script
docker-compose.yml Removed ClickHouse service and volume, updated app env vars for PostHog
railway.template.json Removed ClickHouse service definition, replaced env vars with PostHog configuration

Sequence Diagram

sequenceDiagram
    participant Client as Browser Client
    participant Layout as Next.js Layout
    participant PHProvider as PostHogProvider
    participant PHClient as posthog-js
    participant Server as Elysia Server
    participant ReqCtx as RequestContext
    participant PHNode as posthog-node
    participant PHApi as PostHog API
    participant Admin as Admin Dashboard
    
    Note over Client,PHApi: Client-Side Tracking Flow
    Client->>Layout: Page Load
    Layout->>PHProvider: Initialize Provider
    PHProvider->>PHClient: posthog.init(NEXT_PUBLIC_POSTHOG_KEY)
    PHProvider->>PHClient: capture("$pageview")
    PHClient->>PHApi: Send pageview event
    
    Note over Server,PHApi: Server-Side Analytics Flow
    Client->>Server: API Request (/api/article)
    Server->>ReqCtx: Create request context
    ReqCtx->>ReqCtx: Process request
    ReqCtx->>PHNode: trackEvent(request_event)
    PHNode->>PHNode: Buffer event (flushAt: 50)
    PHNode-->>PHApi: Batch send events
    ReqCtx-->>Client: Response
    
    Note over Server,PHApi: Ad Tracking Flow
    Client->>Server: Ad Request (/api/context)
    Server->>Server: Fetch ads from Gravity
    Client->>Server: Track Impression (/api/px)
    Server->>Server: Forward to Gravity pixel
    Server->>PHNode: trackAdEvent(ad_event)
    PHNode->>PHApi: Send ad event
    Server-->>Client: 200 OK
    
    Note over Admin,PHApi: Dashboard Query Flow
    Client->>Admin: Load Admin Dashboard
    Admin->>PHApi: POST /api/projects/{id}/query/
    Note over Admin,PHApi: HogQL Query:<br/>SELECT properties.hostname<br/>FROM events<br/>WHERE event = 'request_event'
    PHApi-->>Admin: Return results
    Admin-->>Client: Render dashboard
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

9 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link

@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: 4

🤖 Fix all issues with AI agents
In `@components/providers/posthog-provider.tsx`:
- Around line 25-45: PostHogProvider currently renders PHProvider with an
uninitialized posthog client; change PostHogProvider to track initialization
(e.g., useState isInitialized) and only render PHProvider, PostHogPageView and
children after posthog.init(key, { api_host: host, ... }) has completed and the
flag is set; call posthog.init inside the useEffect and set isInitialized true
in the same effect (and guard to avoid re-initializing by checking a posthog
init flag or a local ref) so usePostHog()/PostHogPageView never see an
uninitialized client and double-init in Strict Mode is prevented.

In `@lib/posthog.ts`:
- Around line 168-181: The fetch to `${host}/api/projects/${projectId}/query/`
can hang; wrap it with an AbortController and set a timeout (e.g., 10s or
configurable) that calls controller.abort(), pass signal to fetch, clear the
timeout on success, and handle abort/errors by logging and returning [] instead
of waiting indefinitely; update the fetch invocation that uses host, projectId,
personalApiKey, and query to include the controller.signal and timeout logic and
ensure the timeout is cleared when a response is received.
- Around line 232-239: The current process.on("beforeExit", async () => await
closePostHog()) won’t be awaited by Node so buffered PostHog events may be lost;
replace the beforeExit handler with proper signal handlers (process.on("SIGINT",
...) and process.on("SIGTERM", ...)) that call an async shutdown function which
awaits closePostHog() and guards with the existing isInitialized flag to avoid
double shutdown; ensure the signal handlers call the async shutdown and then
exit (or set exit code) only after closePostHog() completes, and remove/replace
the prior beforeExit registration around isInitialized to use these
SIGINT/SIGTERM handlers instead.

In `@server/routes/admin.ts`:
- Around line 441-455: The LIKE-escaping order is wrong and allows wildcard
injection; change escapeForLike to first escape LIKE wildcards (replace % → \%
and _ → \_) on the raw input, then call escapeStr to escape backslashes and
single-quotes, so functions escapeForLike and escapeStr produce a safe pattern
for properties.url LIKE '%...%'; update the call sites (e.g., where urlSearch is
used in the properties.url LIKE clause) to use the corrected escapeForLike
implementation and consider switching to parameterized queries if supported.
🧹 Nitpick comments (2)
server/routes/admin.ts (1)

508-1247: Consider adding a timeout for the parallel query batch.

This endpoint executes 37 HogQL queries in parallel via Promise.all. While individual query failures are caught for ad queries (.catch(() => [])), there's no overall timeout. If PostHog becomes slow or unresponsive, the admin dashboard request could hang indefinitely.

♻️ Suggested: add a timeout wrapper
// At module level
const withTimeout = <T>(promise: Promise<T>, ms: number, fallback: T): Promise<T> =>
  Promise.race([
    promise,
    new Promise<T>((resolve) => setTimeout(() => resolve(fallback), ms))
  ]);

// Usage example for critical queries
withTimeout(queryPostHog<HostnameStats>(`...`), 30000, [])

Additionally, request event queries (1-12) don't have .catch() handlers, so a single failure will reject the entire Promise.all. Consider adding catch handlers for consistency.

lib/posthog.ts (1)

205-219: Stub function returns misleading zero values.

getBufferStats always returns zeroes, which may mislead monitoring or alerting code that relies on these metrics. Consider adding a comment at call sites or logging a warning if this function is called, to make it clear these stats are unavailable with PostHog.

Comment on lines +25 to +45
export function PostHogProvider({ children }: { children: React.ReactNode }) {
useEffect(() => {
const key = process.env.NEXT_PUBLIC_POSTHOG_KEY;
const host = process.env.NEXT_PUBLIC_POSTHOG_HOST;
if (!key || !host) return;

posthog.init(key, {
api_host: host,
capture_pageview: false, // we handle it manually above
capture_pageleave: true,
});
}, []);

return (
<PHProvider client={posthog}>
<Suspense fallback={null}>
<PostHogPageView />
</Suspense>
{children}
</PHProvider>
);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

posthog-js initialization race condition Next.js PHProvider

💡 Result:

In the common Next.js App Router setup, people put posthog.init(...) inside a useEffect in a PHProvider. That can create an initialization race because useEffect runs after the first client render/hydration, so any code that calls posthog.capture(...), reads feature flags, etc. during the first render (or in effects that run before/without waiting) may run before PostHog is initialized. PostHog’s own docs now prefer initializing earlier via instrumentation-client.ts|js when available. [1][2]

Fix (recommended): initialize in instrumentation-client.ts (Next.js 15.3+)

Create instrumentation-client.ts at the project root and init there (runs early on the client):

// instrumentation-client.ts
import posthog from 'posthog-js'

posthog.init(process.env.NEXT_PUBLIC_POSTHOG_KEY!, {
  api_host: process.env.NEXT_PUBLIC_POSTHOG_HOST,
  defaults: '2025-11-30',
})

This is the official “lightweight, fast” Next.js integration path. [2]

If you must use a PHProvider (Next.js ≤ 15.2 / older pattern)

PostHog’s older doc pattern initializes in a provider useEffect (works, but is where the race comes from). [2] To avoid the race, either:

  1. Init synchronously in a client-only module before rendering children (and keep the provider just for context), or
  2. Gate children rendering until initialized (e.g., with local state), so nothing can call PostHog “too early”.

(If you share your Next.js version + whether you’re on App Router or Pages Router, I can give the exact snippet for your setup.)

Sources: PostHog’s Next.js library docs and Next.js installation docs (including the instrumentation-client.ts|js recommendation and the older useEffect provider pattern). [1][2]

[1] (posthog.com)
[2] (posthog.com)

Citations:


🌐 Web query:

posthog-js init() event queuing before client ready

💡 Result:

In posthog-js, “queueing before the client is ready” depends on how you load it:

1) If you use the standard HTML snippet (recommended)

The snippet creates a stub window.posthog where methods like capture, identify, etc. are functions that push their calls into an array. Once the real SDK (/static/array.js) finishes loading, it replays that queued call list. (posthog.com)

So this works (calls are queued safely even before the SDK is downloaded):

<script>
  // snippet...
  posthog.init('PH_PROJECT_KEY', { api_host: 'https://us.i.posthog.com' })

  // safe immediately: queued by the stub until array.js loads
  posthog.capture('signup_clicked')
</script>

2) If you install via npm (import posthog from 'posthog-js')

There is no stub/global queue unless you build one. You should call posthog.init(...) before calling posthog.capture(...). (posthog.com)

If what you mean by “ready” is “initialized and safe to run my setup (identify, register, etc.)”, use the loaded callback on init:

import posthog from 'posthog-js'

posthog.init('PH_PROJECT_KEY', {
  api_host: 'https://us.i.posthog.com',
  loaded: (ph) => {
    // run things that must wait until init finishes
    ph.identify(userId)
    ph.capture('client_ready')
  },
})

(posthog.com)

3) If “ready” means feature flags are available

Use posthog.onFeatureFlags(...) to run code after flags are loaded (the snippet exposes this API as well). (posthog.com)

Citations:


🌐 Web query:

PostHog React provider pattern "PHProvider" initialization timing

💡 Result:

PHProvider (i.e., PostHogProvider from @posthog/react) does not initialize PostHog for you—it just puts a client instance into React context. So initialization timing is entirely determined by when you call posthog.init(...). (posthog.com)

What this means in practice

  • Pure client-side React (no SSR): initialize before rendering the provider (module/top-level init), then render <PostHogProvider client={posthog}>…</PostHogProvider>. (posthog.com)
  • SSR/hydration frameworks (Next.js App Router, Remix, etc.): you must initialize client-side only.
    • Common pattern: call posthog.init(...) in a useEffect inside a client component, and (optionally) delay rendering the provider until after hydration so nothing consumes an uninitialized client on the first render. (posthog.com)
    • Alternative pattern: guard a top-level init with if (typeof window !== 'undefined') posthog.init(...) so it runs immediately on the client (not waiting for useEffect). (posthog.com)

Why people use a “hydrated gate” in PHProvider

If you render <PostHogProvider client={posthog}> immediately but only call posthog.init in useEffect, then the first client render happens before init, and anything that calls usePostHog() during that render can see an uninitialized client (docs also warn against directly importing the client for this reason). (posthog.com)

Citations:


Fix initialization race condition: PostHog client used before init() completes.

When using the npm posthog-js SDK, there is no automatic event queueing. The code renders PHProvider immediately with an uninitialized posthog client while init() runs asynchronously in useEffect. This means:

  • First render passes an uninitialized client to the provider
  • PostHogPageView may attempt to capture pageviews before initialization completes
  • Components using usePostHog() hook during first render encounter an uninitialized client
  • React Strict Mode (dev) triggers the effect twice, potentially calling init() multiple times

Recommended for Next.js 15.3+: Initialize PostHog in instrumentation-client.ts (runs early, before provider renders), avoiding the race entirely.

For current code: Gate the provider's children until initialization completes:

♻️ Suggested fix: wait for initialization
+import { useState } from 'react'
 
 export function PostHogProvider({ children }: { children: React.ReactNode }) {
+  const [isReady, setIsReady] = useState(false);
+
   useEffect(() => {
     const key = process.env.NEXT_PUBLIC_POSTHOG_KEY;
     const host = process.env.NEXT_PUBLIC_POSTHOG_HOST;
     if (!key || !host) return;
 
     posthog.init(key, {
       api_host: host,
       capture_pageview: false,
       capture_pageleave: true,
+      loaded: () => setIsReady(true),
     });
   }, []);
 
   return (
     <PHProvider client={posthog}>
-      <Suspense fallback={null}>
-        <PostHogPageView />
-      </Suspense>
+      {isReady && (
+        <Suspense fallback={null}>
+          <PostHogPageView />
+        </Suspense>
+      )}
       {children}
     </PHProvider>
   );
 }
🤖 Prompt for AI Agents
In `@components/providers/posthog-provider.tsx` around lines 25 - 45,
PostHogProvider currently renders PHProvider with an uninitialized posthog
client; change PostHogProvider to track initialization (e.g., useState
isInitialized) and only render PHProvider, PostHogPageView and children after
posthog.init(key, { api_host: host, ... }) has completed and the flag is set;
call posthog.init inside the useEffect and set isInitialized true in the same
effect (and guard to avoid re-initializing by checking a posthog init flag or a
local ref) so usePostHog()/PostHogPageView never see an uninitialized client and
double-init in Strict Mode is prevented.

Comment on lines +168 to +181
try {
const response = await fetch(`${host}/api/projects/${projectId}/query/`, {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bearer ${personalApiKey}`,
},
body: JSON.stringify({ query: { kind: "HogQLQuery", query } }),
});

if (!response.ok) {
console.error(`[posthog] HogQL query failed (${response.status}):`, await response.text().catch(() => ""));
return [];
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add a timeout to the HogQL query fetch to prevent indefinite hangs.

The fetch call has no timeout configured. If the PostHog API is slow or unresponsive, this could block indefinitely, degrading the admin dashboard experience or causing upstream timeouts.

🛡️ Proposed fix using AbortController
 export async function queryPostHog<T>(query: string): Promise<T[]> {
   const host = process.env.POSTHOG_HOST;
   const projectId = process.env.POSTHOG_PROJECT_ID;
   const personalApiKey = process.env.POSTHOG_PERSONAL_API_KEY;
 
   if (!host || !projectId || !personalApiKey) return [];
 
+  const controller = new AbortController();
+  const timeoutId = setTimeout(() => controller.abort(), 30_000);
+
   try {
     const response = await fetch(`${host}/api/projects/${projectId}/query/`, {
       method: "POST",
       headers: {
         "Content-Type": "application/json",
         Authorization: `Bearer ${personalApiKey}`,
       },
       body: JSON.stringify({ query: { kind: "HogQLQuery", query } }),
+      signal: controller.signal,
     });
+  } finally {
+    clearTimeout(timeoutId);
+  }
🤖 Prompt for AI Agents
In `@lib/posthog.ts` around lines 168 - 181, The fetch to
`${host}/api/projects/${projectId}/query/` can hang; wrap it with an
AbortController and set a timeout (e.g., 10s or configurable) that calls
controller.abort(), pass signal to fetch, clear the timeout on success, and
handle abort/errors by logging and returning [] instead of waiting indefinitely;
update the fetch invocation that uses host, projectId, personalApiKey, and query
to include the controller.signal and timeout logic and ensure the timeout is
cleared when a response is received.

Comment on lines +232 to +239
// Register shutdown handler
let isInitialized = false;
if (!isInitialized && typeof process !== "undefined") {
isInitialized = true;
process.on("beforeExit", async () => {
await closePostHog();
});
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for all closePostHog calls in the codebase
echo "=== closePostHog calls ==="
rg -n "closePostHog" --type ts

echo -e "\n=== SIGTERM/SIGINT handlers ==="
rg -n "SIGTERM|SIGINT" --type ts

echo -e "\n=== beforeExit references ==="
rg -n "beforeExit" --type ts

Repository: mrmps/SMRY

Length of output: 377


🌐 Web query:

Node.js beforeExit event promise await behavior

💡 Result:

  • process.on('beforeExit', ...) fires only when Node’s event loop becomes empty (no more work scheduled). A beforeExit handler can “make asynchronous calls, and thereby cause the Node.js process to continue” only if it schedules real work (timers, I/O, sockets, etc.). [1]
  • beforeExit handlers are normal EventEmitter listeners: they are called synchronously, and any return value is ignored—so an async handler (a returned Promise) is not awaited by Node. [2]
  • A pending Promise by itself does not keep Node alive; what keeps Node alive is outstanding “ref’ed” event-loop work (e.g., a setTimeout timer keeps the loop running by default). [3][4]

Practical implication

process.on('beforeExit', async () => {
  await Promise.resolve();          // does NOT keep Node alive
  // Node will still exit unless you schedule real work:
  await new Promise(r => setTimeout(r, 10)); // schedules a timer => keeps Node alive
});

Rule of thumb: If your await ultimately waits on something that creates/uses an active handle (timer, network, fs, etc.), the process can stay up; if it’s “just a Promise”, Node may exit anyway. [1][3][4]

Sources: [1] Node.js process docs (beforeExit) https://nodejs.org/api/process.html, [2] Node.js events docs (EventEmitter listeners called synchronously; return values ignored) https://nodejs.org/api/events.html, [3] Node.js timers docs (timers keep the event loop active; ref/unref) https://nodejs.org/api/timers.html, [4] Explanation that Node exits when no async I/O/timers are pending (Promise alone doesn’t keep it alive) https://stackoverflow.com/questions/75857553/why-node-js-does-not-wait-for-promise-to-resolve-before-exiting


🏁 Script executed:

#!/bin/bash
# Read the posthog.ts file to see the full closePostHog implementation
sed -n '220,245p' lib/posthog.ts

# Check if there's any PostHog client definition
echo -e "\n=== PostHog client initialization ==="
sed -n '1,50p' lib/posthog.ts

Repository: mrmps/SMRY

Length of output: 1976


beforeExit does not await async callbacks — buffered analytics events will be lost.

The beforeExit event handler is async, but Node.js does not await the returned promise. client.shutdown() will not complete before process exit, causing pending analytics events to be discarded.

Use SIGTERM/SIGINT handlers instead to ensure graceful shutdown is awaited:

Fix: Use signal handlers for proper shutdown
-// Register shutdown handler
-let isInitialized = false;
-if (!isInitialized && typeof process !== "undefined") {
-  isInitialized = true;
-  process.on("beforeExit", async () => {
-    await closePostHog();
-  });
-}
+// Register shutdown handlers for graceful termination
+if (typeof process !== "undefined") {
+  const shutdown = async () => {
+    await closePostHog();
+    process.exit(0);
+  };
+
+  process.once("SIGTERM", shutdown);
+  process.once("SIGINT", shutdown);
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Register shutdown handler
let isInitialized = false;
if (!isInitialized && typeof process !== "undefined") {
isInitialized = true;
process.on("beforeExit", async () => {
await closePostHog();
});
}
// Register shutdown handlers for graceful termination
if (typeof process !== "undefined") {
const shutdown = async () => {
await closePostHog();
process.exit(0);
};
process.once("SIGTERM", shutdown);
process.once("SIGINT", shutdown);
}
🤖 Prompt for AI Agents
In `@lib/posthog.ts` around lines 232 - 239, The current process.on("beforeExit",
async () => await closePostHog()) won’t be awaited by Node so buffered PostHog
events may be lost; replace the beforeExit handler with proper signal handlers
(process.on("SIGINT", ...) and process.on("SIGTERM", ...)) that call an async
shutdown function which awaits closePostHog() and guards with the existing
isInitialized flag to avoid double shutdown; ensure the signal handlers call the
async shutdown and then exit (or set exit code) only after closePostHog()
completes, and remove/replace the prior beforeExit registration around
isInitialized to use these SIGINT/SIGTERM handlers instead.

Comment on lines +441 to 455
const escapeStr = (str: string) => str.replace(/\\/g, "\\\\").replace(/'/g, "''");
const escapeForLike = (str: string) =>
escapeStr(str).replace(/%/g, "\\%").replace(/_/g, "\\_");
if (hostnameFilter) {
conditions.push(`hostname = '${escapeForClickhouse(hostnameFilter)}'`);
conditions.push(`properties.hostname = '${escapeStr(hostnameFilter)}'`);
}
if (sourceFilter) {
conditions.push(`source = '${escapeForClickhouse(sourceFilter)}'`);
conditions.push(`properties.source = '${escapeStr(sourceFilter)}'`);
}
if (outcomeFilter) {
conditions.push(`outcome = '${escapeForClickhouse(outcomeFilter)}'`);
conditions.push(`properties.outcome = '${escapeStr(outcomeFilter)}'`);
}
if (urlSearch) {
conditions.push(`url LIKE '%${escapeForClickhouseLike(urlSearch)}%'`);
conditions.push(`properties.url LIKE '%${escapeForLike(urlSearch)}%'`);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

SQL injection vulnerability in LIKE pattern escaping.

The escapeForLike function has an incorrect escaping order that allows LIKE wildcard injection. It escapes SQL characters first (\\\), then adds LIKE escapes (%\%). This creates a vulnerability:

Attack example:

  • Input: test\%
  • After escapeStr: test\\%
  • After escapeForLike: test\\\%
  • In LIKE clause: \\ = literal backslash, % = wildcard (unescaped!)

An attacker can bypass the intended exact-match filtering by including backslashes before % or _ characters.

🔒 Proposed fix: correct escaping order
-        const escapeStr = (str: string) => str.replace(/\\/g, "\\\\").replace(/'/g, "''");
-        const escapeForLike = (str: string) =>
-          escapeStr(str).replace(/%/g, "\\%").replace(/_/g, "\\_");
+        // For LIKE patterns: first escape LIKE special chars, then escape SQL string chars
+        const escapeLikeChars = (str: string) =>
+          str.replace(/\\/g, "\\\\").replace(/%/g, "\\%").replace(/_/g, "\\_");
+        const escapeStr = (str: string) => str.replace(/\\/g, "\\\\").replace(/'/g, "''");
+        const escapeForLike = (str: string) => escapeStr(escapeLikeChars(str));

Alternatively, consider using parameterized queries if PostHog's HogQL API supports them, which would eliminate injection risks entirely.

🧰 Tools
🪛 GitHub Check: CodeQL

[failure] 443-443: Incomplete string escaping or encoding
This does not escape backslash characters in the input.


[failure] 443-443: Incomplete string escaping or encoding
This does not escape backslash characters in the input.

🤖 Prompt for AI Agents
In `@server/routes/admin.ts` around lines 441 - 455, The LIKE-escaping order is
wrong and allows wildcard injection; change escapeForLike to first escape LIKE
wildcards (replace % → \% and _ → \_) on the raw input, then call escapeStr to
escape backslashes and single-quotes, so functions escapeForLike and escapeStr
produce a safe pattern for properties.url LIKE '%...%'; update the call sites
(e.g., where urlSearch is used in the properties.url LIKE clause) to use the
corrected escapeForLike implementation and consider switching to parameterized
queries if supported.

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.

1 participant