feat: telemetry and security hardening #55
Conversation
…on guide This commit introduces a detailed PostHog instrumentation blueprint for the OpenChat monorepo, covering web, server, and extension components. It outlines baseline setup, event taxonomy with triggers and properties, common properties, dashboards and insights strategies, implementation notes by file, and next steps for full analytics coverage. Key additions include event naming conventions, super-property registrations, enhanced event properties for auth, chat, settings, sync, and extension features, as well as recommended dashboard KPIs and monitoring plans to track acquisition, onboarding, chat health, personalization, and extension engagement. This documentation will guide engineering alignment and implementation to enable actionable analytics through PostHog. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
…trumentation - Reduced and optimized core event sets to focus on high-leverage insights, minimizing low-signal noise. - Standardized event naming conventions to lowercase snake_case with scope.action structure. - Updated super-properties registration for better session identification including guest handling. - Enhanced event properties for chat creation, streaming, attachments, and OpenRouter API key lifecycle. - Added new telemetry events for sync connection state, fallback storage usage, and chat rate limiting. - Revised dashboard and funnel definitions to reflect streamlined event model and metrics. - Improved implementation notes for consistency and expanded coverage across web, server, and extension components. This update enables effective monitoring of acquisition, chat usage, reliability, and onboarding activation without excess event volume. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
- Add base super properties and sanitize event properties for client and server PostHog usage - Track fallback storage usage in server router for chats and messages - Add detailed client event tracking in web chat handler and components - Introduce PosthogBootstrap component for consistent client identification and property registration - Enhance event tracking with user workspace IDs, auth states, theme info, and OpenRouter API key status - Track marketing CTA clicks and landing visits with detailed metadata - Improve rate limit and error event captures for chat and OpenRouter APIs - Add client property registration on model selection and OpenRouter key management - Add sync connection state events to PostHog This update improves analytics granularity and consistency across client and server, enabling better user behavior insights and troubleshooting. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR implements comprehensive PostHog telemetry tracking and security hardening across the OpenChat monorepo. The changes add structured analytics to track user behavior throughout the application lifecycle - from landing page visits and CTA clicks to chat operations, API key management, and WebSocket connection states. Key telemetry additions include event tracking in 14+ components with consistent snake_case property naming, user identification with workspace grouping, and detailed error/performance monitoring.
The security improvements focus on two critical areas: (1) XSS protection through comprehensive character escaping for user data injected into inline JavaScript, and (2) strengthening OpenRouter API key encryption by replacing SHA-256 hashing with PBKDF2 using 100,000 iterations. The implementation includes property sanitization throughout to prevent undefined values from reaching PostHog, environment-aware configuration for different deployment contexts, and proper client identification patterns.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
| apps/web/src/app/dashboard/layout.tsx | 4/5 | Adds comprehensive XSS protection by escaping user data before JavaScript injection |
| apps/server/src/lib/openrouter.ts | 3/5 | Replaces SHA-256 with PBKDF2 for stronger API key encryption but lacks migration strategy |
| apps/web/src/components/hero-section.tsx | 3/5 | Converts server component to client-side for telemetry but may impact SEO/performance |
| apps/web/src/lib/posthog.ts | 4/5 | Implements comprehensive client-side PostHog integration with property sanitization |
| apps/server/src/lib/posthog.ts | 5/5 | Robust server-side telemetry setup with environment-aware configuration |
| apps/web/src/components/posthog-bootstrap.tsx | 5/5 | Well-architected telemetry bootstrap component with proper cleanup handling |
| posthog.md | 5/5 | Comprehensive telemetry documentation with clear event taxonomy and implementation guide |
| apps/web/src/components/chat-room.tsx | 4/5 | Extensive telemetry integration throughout chat flow with duplicate prevention |
| apps/server/src/routers/index.ts | 4/5 | Server-side chat telemetry with fallback storage tracking |
| apps/web/src/app/api/chat/chat-handler.ts | 4/5 | Chat API telemetry with IP hashing and rate limiting tracking |
| apps/web/src/components/app-sidebar.tsx | 4/5 | Sidebar telemetry for user identification and dashboard entry tracking |
| apps/web/src/components/chat-composer.tsx | 4/5 | File attachment telemetry with privacy-conscious textarea exclusion |
| apps/web/src/components/model-selector.tsx | 4/5 | Model selection tracking via PostHog property registration |
| apps/web/src/lib/sync.ts | 4/5 | WebSocket connection state telemetry for monitoring sync reliability |
| apps/web/src/components/account-settings-modal.tsx | 4/5 | OpenRouter API key management telemetry with masked data |
| apps/web/src/components/openrouter-link-modal.tsx | 4/5 | API key prompt telemetry with deduplication logic |
| apps/web/src/components/providers.tsx | 4/5 | Enhanced pageview tracking with referrer and entry path data |
| apps/web/src/lib/posthog-server.ts | 4/5 | Server-side PostHog configuration with environment variable fallbacks |
Confidence score: 3/5
- This PR requires careful review due to security changes and potential breaking changes from the OpenRouter encryption update
- Score reflects the mix of well-implemented telemetry features alongside concerning security changes - the PBKDF2 upgrade lacks migration strategy for existing encrypted keys, hero section client-side conversion may impact performance, and the fixed salt in PBKDF2 reduces security benefits
- Pay close attention to apps/server/src/lib/openrouter.ts for the encryption migration issue, apps/web/src/app/dashboard/layout.tsx for XSS protection implementation, and apps/web/src/components/hero-section.tsx for the server-to-client component conversion
Sequence Diagram
sequenceDiagram
participant User
participant Web as "Web App"
participant Server as "Server API"
participant PostHog as "PostHog Analytics"
participant OpenRouter as "OpenRouter API"
User->>Web: "Access dashboard"
Web->>PostHog: "captureClientEvent('dashboard.entered')"
Note over Web: Bootstrap telemetry with user context
User->>Web: "Add OpenRouter API key"
Web->>PostHog: "captureClientEvent('openrouter.key_prompt_shown')"
Web->>Web: "Encrypt API key (AES-256-GCM + PBKDF2)"
Web->>PostHog: "captureClientEvent('openrouter.key_saved')"
User->>Web: "Create new chat"
Web->>Server: "POST /chats/create"
Server->>PostHog: "capturePosthogEvent('chat.created')"
Server-->>Web: "Chat created response"
Web->>PostHog: "captureClientEvent('chat.created')"
User->>Web: "Send message"
Web->>Server: "POST /api/chat"
Note over Server: Rate limiting check with IP hashing
Server->>PostHog: "captureServerEvent('chat_message_stream')"
Server->>Server: "Decrypt API key with hardened encryption"
Server->>OpenRouter: "Stream chat request"
OpenRouter-->>Server: "Response stream"
Server-->>Web: "Streamed response"
Web->>PostHog: "captureClientEvent('chat_message_submitted')"
alt Database unavailable
Server->>Server: "Fall back to in-memory storage"
Server->>PostHog: "capturePosthogEvent('workspace.fallback_storage_used')"
end
alt Rate limit exceeded
Server->>PostHog: "captureServerEvent('chat.rate_limited')"
Server-->>Web: "429 Too Many Requests"
Web->>PostHog: "captureClientEvent('chat.rate_limited')"
end
User->>Web: "Upload attachment"
Web->>Web: "Validate file size and type"
Web->>PostHog: "captureClientEvent('chat.attachment_event')"
User->>Web: "Visit marketing page"
Web->>PostHog: "captureClientEvent('marketing.visit_landing')"
User->>Web: "Click CTA button"
Web->>PostHog: "captureClientEvent('marketing.cta_clicked')"
Additional Comments (1)
-
apps/server/src/lib/openrouter.ts, line 65-74 (link)logic: This change breaks backward compatibility - existing encrypted API keys cannot be decrypted with the new key derivation method
Context used:
- Context from
dashboard- AGENTS.md (source)
18 files reviewed, 11 comments
| throw new Error("Missing OPENROUTER_API_KEY_SECRET env for encrypting OpenRouter API keys"); | ||
| } | ||
| return createHash("sha256").update(secret).digest(); | ||
| const salt = "openrouter:key-derivation-salt"; |
There was a problem hiding this comment.
style: Using a fixed salt reduces security - consider generating unique salts per key or per user to prevent rainbow table attacks
| setInternalValue(currentValue) | ||
| } | ||
| onChange?.(currentValue) | ||
| registerClientProperties({ model_id: currentValue }) |
There was a problem hiding this comment.
style: tracks model selection when user explicitly chooses from dropdown - this creates duplicate tracking when both useEffect and onSelect fire for the same selection
| process.env.POSTHOG_DEPLOYMENT_REGION ?? process.env.VERCEL_REGION ?? "local"; | ||
|
|
||
| const BASE_SUPER_PROPERTIES = Object.freeze({ | ||
| app: "openchat-server", |
There was a problem hiding this comment.
logic: The app name is hardcoded as 'openchat-server' but this is in the web app's lib directory - should this be 'openchat-web'?
| toast.success("OpenRouter key saved"); | ||
| captureClientEvent("openrouter.key_saved", { | ||
| source: "settings", | ||
| masked_tail: trimmed.slice(-4), |
There was a problem hiding this comment.
style: Exposing the last 4 characters of API key in telemetry could be a privacy concern, even though masked
| if (char === "\\" || char === "]" || char === "^" || char === "-") { | ||
| return `\\${char}`; | ||
| } | ||
| if (char === "/") return "\\/"; |
There was a problem hiding this comment.
style: Redundant handling: line 32 specifically handles '/' but it's already covered in the charMap on line 15
| const workspaceId = options?.workspaceId; | ||
| if (workspaceId) { | ||
| client.group("workspace", workspaceId); | ||
| registerClientProperties({ workspace_id: workspaceId }); |
There was a problem hiding this comment.
style: The workspace_id property registration here creates a permanent super property - consider if this should be cleared when switching workspaces
| identifyClient(currentUserId, { | ||
| workspaceId: currentUserId, | ||
| properties: { auth_state: session?.user ? "member" : "guest" }, | ||
| }); | ||
| registerClientProperties({ | ||
| auth_state: session?.user ? "member" : "guest", | ||
| workspace_id: currentUserId, | ||
| }); |
There was a problem hiding this comment.
style: User identification happens on every render when currentUserId or session.user changes. Consider if this could cause excessive PostHog API calls.
| const dashboardTrackedRef = useRef(false); | ||
| useEffect(() => { | ||
| if (dashboardTrackedRef.current) return; | ||
| if (isLoading) return; | ||
| dashboardTrackedRef.current = true; | ||
| void (async () => { | ||
| let hasKey = false; | ||
| try { | ||
| const key = await loadOpenRouterKey(); | ||
| hasKey = Boolean(key); | ||
| registerClientProperties({ has_openrouter_key: hasKey }); | ||
| } catch { | ||
| hasKey = false; | ||
| } | ||
| const entryPath = typeof window !== "undefined" ? window.location.pathname || "/dashboard" : "/dashboard"; | ||
| captureClientEvent("dashboard.entered", { | ||
| chat_total: baseChats.length, | ||
| has_api_key: hasKey, | ||
| entry_path: entryPath, | ||
| brand_theme: brandTheme, | ||
| }); | ||
| })(); | ||
| }, [baseChats.length, brandTheme, isLoading]); |
There was a problem hiding this comment.
logic: Dashboard tracking uses a ref to prevent duplicate events, but the effect depends on baseChats.length which could cause re-runs when chats change.
| @@ -1,11 +1,15 @@ | |||
| import React from 'react' | |||
| "use client"; | |||
There was a problem hiding this comment.
style: converting the hero section to client-side rendering may impact SEO and initial page load performance for this above-the-fold content
|
|
||
| useEffect(() => { | ||
| if (visitTrackedRef.current) return | ||
| if (typeof session === 'undefined') return |
There was a problem hiding this comment.
logic: the session check typeof session === 'undefined' may not handle loading states properly - consider checking for session === null for unauthenticated users
Summary
Testing