feat: telemetry and security hardening #57
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 telemetry infrastructure and security hardening across the OpenChat application. The changes introduce PostHog analytics integration throughout the web and server applications, tracking user interactions, chat operations, API usage patterns, and system performance metrics. Key telemetry additions include user identification, workspace grouping, model selection tracking, chat creation/messaging events, and WebSocket connection monitoring.
The security improvements focus on two critical areas: upgrading OpenRouter API key encryption from weak SHA-256 to PBKDF2 with 100,000 iterations (while maintaining backward compatibility), and implementing robust XSS prevention for user ID injection in client-side scripts through comprehensive character escaping. Additional security measures include IP address hashing for privacy-compliant rate limiting and preventing PostHog from capturing sensitive textarea content.
The implementation follows React best practices with proper cleanup patterns, useRef-based deduplication, and error handling. Environment-based configuration enables different tracking behavior across deployment contexts (local, staging, production) while maintaining consistent metadata through base super properties.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
apps/web/src/components/openrouter-link-modal.tsx |
4/5 | Added PostHog telemetry to track OpenRouter API key prompts with context about errors vs missing keys |
apps/web/src/components/providers.tsx |
4/5 | Enhanced PostHog provider with pageview tracking including referrer analysis and PosthogBootstrap integration |
apps/web/src/components/model-selector.tsx |
4/5 | Added model selection tracking to register currently selected AI model as client property |
apps/web/src/lib/posthog-server.ts |
4/5 | Enhanced server-side PostHog with environment detection, base properties, and property sanitization |
apps/server/src/lib/posthog.ts |
4/5 | Implemented PostHog client for server with environment-based config and property sanitization |
apps/server/src/routers/index.ts |
5/5 | Added comprehensive telemetry for chat operations and fallback storage usage tracking |
apps/web/src/components/account-settings-modal.tsx |
4/5 | Added telemetry tracking for OpenRouter API key save/remove operations with masked key tails |
apps/web/src/lib/sync.ts |
4/5 | Added WebSocket connection lifecycle telemetry tracking with retry counts and tab IDs |
apps/web/src/components/posthog-bootstrap.tsx |
4/5 | New component handling client-side telemetry collection and user identification with workspace context |
apps/web/src/lib/posthog.ts |
4/5 | Enhanced client-side PostHog with base properties, sanitization, and workspace grouping |
apps/server/src/lib/openrouter.ts |
4/5 | Security hardening: upgraded encryption from SHA-256 to PBKDF2 with backward compatibility |
apps/web/src/components/chat-room.tsx |
3/5 | Comprehensive telemetry for chat interactions with potential security concerns around data exposure |
apps/web/src/components/chat-composer.tsx |
4/5 | Added file attachment telemetry and privacy protection for textarea content |
apps/web/src/app/dashboard/layout.tsx |
4/5 | Security hardening for user ID injection with comprehensive character escaping |
apps/web/src/components/hero-section.tsx |
4/5 | Added marketing analytics for landing page visits and CTA tracking |
apps/web/src/components/app-sidebar.tsx |
4/5 | Comprehensive sidebar telemetry including user identification and dashboard entry tracking |
apps/web/src/app/api/chat/chat-handler.ts |
4/5 | Added IP hashing, enhanced rate limiting, and comprehensive chat stream telemetry |
Confidence score: 3/5
- This PR introduces significant complexity with potential security and performance implications that require careful review
- Score reflects concerns about sensitive data exposure in telemetry events (API key tails, error messages), potential XSS risks from extensive client-side tracking, and performance impact from comprehensive analytics
- Pay close attention to
apps/web/src/components/chat-room.tsxfor data exposure issues,apps/server/src/lib/openrouter.tsfor encryption implementation, andapps/web/src/app/dashboard/layout.tsxfor XSS prevention effectiveness
Sequence Diagram
sequenceDiagram
participant User
participant Web App
participant Server
participant PostHog
participant Database
participant OpenRouter
User->>Web App: Access dashboard
Web App->>Web App: Load OpenRouter API key from storage
Web App->>Server: Fetch models (if key exists)
Server->>OpenRouter: Request available models
OpenRouter-->>Server: Return model list
Server-->>Web App: Return model list
Web App->>Web App: Display chat interface
User->>Web App: Enter message and select model
Web App->>Server: POST /api/chat with message, model, API key
Server->>Server: Validate request and rate limits
Server->>PostHog: Capture chat_message_stream event
Server->>Database: Store user message
Server->>OpenRouter: Stream request with user message
OpenRouter-->>Server: Stream AI response
Server->>Server: Persist assistant message chunks
Server-->>Web App: Stream response back to UI
Web App->>Web App: Display streaming response
Server->>PostHog: Capture completion metrics
Server->>Database: Store final assistant message
Web App->>PostHog: Capture client-side events
Context used:
- Context from
dashboard- AGENTS.md (source)
17 files reviewed, 17 comments
| const reason = errorMessage ? "error" : "missing"; | ||
| captureClientEvent("openrouter.key_prompt_shown", { | ||
| reason, | ||
| has_api_key: Boolean(hasApiKey), |
There was a problem hiding this comment.
style: Boolean() wrapper is unnecessary since hasApiKey is already boolean | undefined
| if (client) { | ||
| setPosthogClient(client); | ||
| client.capture("$pageview"); | ||
| const referrerUrl = document.referrer && document.referrer.length > 0 ? document.referrer : "direct"; |
There was a problem hiding this comment.
style: Consider using optional chaining: document.referrer?.length > 0 instead of document.referrer && document.referrer.length > 0
| } catch { | ||
| referrerDomain = "direct"; | ||
| } |
There was a problem hiding this comment.
style: The empty catch block silently ignores URL parsing errors. Consider logging the error for debugging purposes in development
| setInternalValue(currentValue) | ||
| } | ||
| onChange?.(currentValue) | ||
| registerClientProperties({ model_id: currentValue }) |
There was a problem hiding this comment.
style: Duplicate tracking call - already handled by useEffect on line 94-97 when selectedValue changes
| 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 'openchat-server' seems incorrect for the web app context - this file is in apps/web but references server
| registerClientProperties({ has_openrouter_key: true }); | ||
| captureClientEvent("openrouter.key_saved", { | ||
| source: "modal", | ||
| masked_tail: key.slice(-4), |
There was a problem hiding this comment.
style: Exposing last 4 characters of API key in telemetry could be a security risk if logs are compromised
| error_message: errorMessage, | ||
| provider_host: providerHost, | ||
| has_api_key: Boolean(key), |
There was a problem hiding this comment.
style: Error messages from external APIs might contain sensitive information that shouldn't be logged
| auth_state: session?.user ? "member" : "guest", | ||
| workspace_id: workspaceId ?? identifier, | ||
| }); | ||
| }, [memoDevUser, session?.user, session?.user?.id, workspaceId]); |
There was a problem hiding this comment.
logic: Missing workspaceId in dependency array could cause stale closures
| const { data: session } = authClient.useSession() | ||
| const visitTrackedRef = useRef(false) |
There was a problem hiding this comment.
style: Session data is used before being fully loaded. Consider adding loading state check.
|
|
||
| useEffect(() => { | ||
| if (visitTrackedRef.current) return | ||
| if (typeof session === 'undefined') return |
There was a problem hiding this comment.
logic: This condition may prevent tracking when session is loading (undefined vs null)
|
Thanks for the thorough pass! To address the security notes:
If you spot particular call sites still worrying you (e.g. specific telemetry properties), let me know and I can further lock them down. |
|
Quick follow-up with the literal identifiers since the previous comment stripped backticks:
Happy to address anything else you spot. |
fix(server): require env key for legacy openrouter
docs(server): note legacy key env
1 participant
Summary
Testing