Fix Mixpanel analytics review findings FRE-5281

P0: Fix validation bypass - validated properties now override raw properties
P1: Add unit tests for shared-analytics package (3 test files)
P1: Refactor spamshield to use shared-analytics, deprecate duplicate
P2: Normalize phone numbers to E.164 before hashing
P2: Add graceful error handling for missing env vars in config
P3: Add singleton pattern to MixpanelService
P3: Include timestamp in validated properties schema
This commit is contained in:
2026-05-17 15:37:21 -04:00
parent 986941e201
commit 06ca3ec0cf
10 changed files with 494 additions and 32 deletions

View File

@@ -6,7 +6,10 @@
"main": "src/index.ts", "main": "src/index.ts",
"types": "src/index.ts", "types": "src/index.ts",
"scripts": { "scripts": {
"lint": "eslint src/" "lint": "eslint src/",
"test": "vitest run",
"test:watch": "vitest",
"typecheck": "tsc --noEmit"
}, },
"dependencies": { "dependencies": {
"@segment/analytics-node": "^1.0.0", "@segment/analytics-node": "^1.0.0",
@@ -14,6 +17,8 @@
"zod": "^4.3.6" "zod": "^4.3.6"
}, },
"devDependencies": { "devDependencies": {
"typescript": "^5.3.3" "typescript": "^5.3.3",
"vitest": "^4.1.5",
"@vitest/coverage-v8": "^4.1.5"
} }
} }

View File

@@ -0,0 +1,113 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { z } from 'zod';
describe('eventPropertiesSchema', () => {
let eventPropertiesSchema: z.ZodType;
beforeEach(async () => {
const config = await import('../config/analytics.config');
eventPropertiesSchema = config.eventPropertiesSchema;
});
it('validates valid properties', () => {
const result = eventPropertiesSchema.parse({
userId: 'user-123',
sessionId: 'session-456',
platform: 'web',
version: '1.0.0',
});
expect(result.userId).toBe('user-123');
expect(result.platform).toBe('web');
});
it('accepts empty properties', () => {
const result = eventPropertiesSchema.parse({});
expect(result).toEqual({});
});
it('accepts null properties', () => {
const result = eventPropertiesSchema.parse(null);
expect(result).toEqual({});
});
it('validates timestamp as Date object', () => {
const now = new Date();
const result = eventPropertiesSchema.parse({ timestamp: now });
expect(result.timestamp).toBe(now);
});
it('validates timestamp as ISO string', () => {
const isoString = '2026-01-01T00:00:00.000Z';
const result = eventPropertiesSchema.parse({ timestamp: isoString });
expect(result.timestamp).toBe(isoString);
});
it('allows extra properties via passthrough', () => {
const result = eventPropertiesSchema.parse({
plan: 'pro',
referrer: 'google',
mrr: 29.99,
});
expect(result.plan).toBe('pro');
expect(result.referrer).toBe('google');
expect(result.mrr).toBe(29.99);
});
it('validates platform enum', () => {
expect(() => eventPropertiesSchema.parse({ platform: 'web' })).not.toThrow();
expect(() => eventPropertiesSchema.parse({ platform: 'mobile' })).not.toThrow();
expect(() => eventPropertiesSchema.parse({ platform: 'desktop' })).not.toThrow();
expect(() => eventPropertiesSchema.parse({ platform: 'api' })).not.toThrow();
expect(() => eventPropertiesSchema.parse({ platform: 'invalid' })).toThrow();
});
});
describe('EventType enum', () => {
let EventType: any;
beforeEach(async () => {
const config = await import('../config/analytics.config');
EventType = config.EventType;
});
it('contains all user events', () => {
expect(EventType.USER_SIGNED_UP).toBe('user_signed_up');
expect(EventType.USER_LOGGED_IN).toBe('user_logged_in');
expect(EventType.USER_LOGGED_OUT).toBe('user_logged_out');
expect(EventType.USER_UPGRADED).toBe('user_upgraded');
expect(EventType.USER_DOWNGRADED).toBe('user_downgraded');
});
it('contains all subscription events', () => {
expect(EventType.SUBSCRIPTION_CREATED).toBe('subscription_created');
expect(EventType.SUBSCRIPTION_UPDATED).toBe('subscription_updated');
expect(EventType.SUBSCRIPTION_CANCELLED).toBe('subscription_cancelled');
expect(EventType.SUBSCRIPTION_RENEWED).toBe('subscription_renewed');
});
it('contains all spam events', () => {
expect(EventType.CALL_ANALYZED).toBe('call_analyzed');
expect(EventType.SMS_ANALYZED).toBe('sms_analyzed');
expect(EventType.SPAM_BLOCKED).toBe('spam_blocked');
expect(EventType.SPAM_FLAGGED).toBe('spam_flagged');
expect(EventType.SPAM_FEEDBACK_SUBMITTED).toBe('spam_feedback_submitted');
});
it('contains all KPI events', () => {
expect(EventType.MRR_UPDATED).toBe('mrr_updated');
expect(EventType.CONVERSION_OCCURRED).toBe('conversion_occurred');
expect(EventType.CHURN_OCCURRED).toBe('churn_occurred');
expect(EventType.REFERRAL_SENT).toBe('referral_sent');
expect(EventType.REFERRAL_CONVERTED).toBe('referral_converted');
});
});
describe('isAnalyticsConfigured', () => {
it('exports configuration status flag', async () => {
const config = await import('../config/analytics.config');
expect(typeof config.isAnalyticsConfigured).toBe('boolean');
});
});

View File

@@ -0,0 +1,180 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
describe('MixpanelService', () => {
let MixpanelService: any;
let mixpanelService: any;
let mockAnalytics: any;
beforeEach(() => {
vi.stubEnv('MIXPANEL_TOKEN', 'test-token');
vi.stubEnv('GA4_MEASUREMENT_ID', 'G-TEST123');
mockAnalytics = {
track: vi.fn(),
identify: vi.fn(),
group: vi.fn(),
flush: vi.fn(),
};
vi.doMock('@segment/analytics-node', () => ({
Analytics: vi.fn(() => mockAnalytics),
}));
vi.resetModules();
});
afterEach(async () => {
vi.unstubAllEnvs();
vi.restoreAllMocks();
const config = await import('../config/analytics.config');
if (config.analyticsEnv) {
vi.clearAllMocks();
}
});
it('implements singleton pattern via getInstance', async () => {
const mod = await import('../services/mixpanel.service');
MixpanelService = mod.MixpanelService;
const instance1 = MixpanelService.getInstance();
const instance2 = MixpanelService.getInstance();
expect(instance1).toBe(instance2);
});
it('exports singleton instance as mixpanelService', async () => {
const mod = await import('../services/mixpanel.service');
mixpanelService = mod.mixpanelService;
const instance = MixpanelService.getInstance();
expect(mixpanelService).toBe(instance);
});
it('validated properties override raw properties (P0 fix)', async () => {
const mod = await import('../services/mixpanel.service');
mixpanelService = mod.mixpanelService;
await mixpanelService.track(
mod.EventType.USER_SIGNED_UP,
'user-123',
{
platform: 'web',
version: 'malicious-value',
}
);
const trackCall = mockAnalytics.track.mock.calls[0];
const properties = trackCall[0].properties;
expect(properties.platform).toBe('web');
expect(properties.version).toBe('malicious-value');
expect(properties.timestamp).toBeDefined();
});
it('adds timestamp to all tracked events', async () => {
const mod = await import('../services/mixpanel.service');
mixpanelService = mod.mixpanelService;
await mixpanelService.track(mod.EventType.USER_SIGNED_UP, 'user-123', {});
const trackCall = mockAnalytics.track.mock.calls[0];
expect(trackCall[0].properties.timestamp).toBeInstanceOf(Date);
});
it('tracks user signup event correctly', async () => {
const mod = await import('../services/mixpanel.service');
mixpanelService = mod.mixpanelService;
await mixpanelService.userSignedUp('user-123', 'pro', 'google');
const trackCall = mockAnalytics.track.mock.calls[0];
expect(trackCall[0].event).toBe('user_signed_up');
expect(trackCall[0].distinctId).toBe('user-123');
expect(trackCall[0].properties.plan).toBe('pro');
expect(trackCall[0].properties.referrer).toBe('google');
});
it('tracks user upgrade event correctly', async () => {
const mod = await import('../services/mixpanel.service');
mixpanelService = mod.mixpanelService;
await mixpanelService.userUpgraded('user-123', 'free', 'pro', 29.99);
const trackCall = mockAnalytics.track.mock.calls[0];
expect(trackCall[0].event).toBe('user_upgraded');
expect(trackCall[0].properties.fromTier).toBe('free');
expect(trackCall[0].properties.toTier).toBe('pro');
expect(trackCall[0].properties.mrr).toBe(29.99);
});
it('tracks spam blocked event with hashed phone number', async () => {
const mod = await import('../services/mixpanel.service');
mixpanelService = mod.mixpanelService;
await mixpanelService.spamBlocked('user-123', '+14155552671', 0.95, 'ml');
const trackCall = mockAnalytics.track.mock.calls[0];
expect(trackCall[0].event).toBe('spam_blocked');
expect(trackCall[0].properties.phoneNumber).toMatch(/^sha256_/);
expect(trackCall[0].properties.confidence).toBe(0.95);
expect(trackCall[0].properties.method).toBe('ml');
});
it('does not send raw phone number in spam blocked events', async () => {
const mod = await import('../services/mixpanel.service');
mixpanelService = mod.mixpanelService;
const rawPhone = '+14155552671';
await mixpanelService.spamBlocked('user-123', rawPhone, 0.95, 'ml');
const trackCall = mockAnalytics.track.mock.calls[0];
expect(trackCall[0].properties.phoneNumber).not.toBe(rawPhone);
});
it('calls identify correctly', async () => {
const mod = await import('../services/mixpanel.service');
mixpanelService = mod.mixpanelService;
await mixpanelService.identify('user-123', { name: 'John Doe', plan: 'pro' });
expect(mockAnalytics.identify).toHaveBeenCalledWith({
distinctId: 'user-123',
traits: { name: 'John Doe', plan: 'pro' },
});
});
it('calls group correctly', async () => {
const mod = await import('../services/mixpanel.service');
mixpanelService = mod.mixpanelService;
await mixpanelService.group('org-123', 'organization', { name: 'Acme Corp' });
expect(mockAnalytics.group).toHaveBeenCalledWith({
groupKey: 'organization',
groupId: 'org-123',
traits: { name: 'Acme Corp' },
});
});
it('calls flush correctly', async () => {
const mod = await import('../services/mixpanel.service');
mixpanelService = mod.mixpanelService;
await mixpanelService.flush();
expect(mockAnalytics.flush).toHaveBeenCalled();
});
it('handles exposure detected event', async () => {
const mod = await import('../services/mixpanel.service');
mixpanelService = mod.mixpanelService;
await mixpanelService.exposureDetected('user-123', 'breach', 'high', 'haveibeenpwned');
const trackCall = mockAnalytics.track.mock.calls[0];
expect(trackCall[0].event).toBe('exposure_detected');
expect(trackCall[0].properties.exposureType).toBe('breach');
expect(trackCall[0].properties.severity).toBe('high');
expect(trackCall[0].properties.source).toBe('haveibeenpwned');
});
});

View File

@@ -0,0 +1,74 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { hashPhoneNumber } from '../utils/phone-hash';
import crypto from 'crypto';
describe('hashPhoneNumber', () => {
it('produces deterministic hash for same phone number', () => {
const hash1 = hashPhoneNumber('+14155552671');
const hash2 = hashPhoneNumber('+14155552671');
expect(hash1).toBe(hash2);
});
it('normalizes different formats to same hash - US numbers', () => {
const hash1 = hashPhoneNumber('+14155552671');
const hash2 = hashPhoneNumber('4155552671');
const hash3 = hashPhoneNumber('(415) 555-2671');
const hash4 = hashPhoneNumber('415-555-2671');
const hash5 = hashPhoneNumber('415.555.2671');
expect(hash1).toBe(hash2);
expect(hash1).toBe(hash3);
expect(hash1).toBe(hash4);
expect(hash1).toBe(hash5);
});
it('normalizes international numbers', () => {
const hash1 = hashPhoneNumber('+442071234567');
const hash2 = hashPhoneNumber('442071234567');
const hash3 = hashPhoneNumber('44 20 7123 4567');
expect(hash1).toBe(hash2);
expect(hash1).toBe(hash3);
});
it('produces SHA-256 hash with prefix', () => {
const hash = hashPhoneNumber('+14155552671');
expect(hash).toMatch(/^sha256_[0-9a-f]{64}$/);
});
it('strips non-digit characters before hashing', () => {
const cleanHash = hashPhoneNumber('+14155552671');
const dirtyHash = hashPhoneNumber('+1 (415) 555-2671 x123');
expect(cleanHash).toBe(dirtyHash);
});
it('handles 10-digit US numbers by adding country code', () => {
const withCountryCode = hashPhoneNumber('+14155552671');
const withoutCountryCode = hashPhoneNumber('4155552671');
expect(withCountryCode).toBe(withoutCountryCode);
});
it('handles 11-digit US numbers starting with 1', () => {
const withPlus = hashPhoneNumber('+14155552671');
const withoutPlus = hashPhoneNumber('14155552671');
expect(withPlus).toBe(withoutPlus);
});
it('different phone numbers produce different hashes', () => {
const hash1 = hashPhoneNumber('+14155552671');
const hash2 = hashPhoneNumber('+442071234567');
expect(hash1).not.toBe(hash2);
});
it('hash matches expected SHA-256 of normalized input', () => {
const normalized = '+14155552671';
const expectedHash = crypto.createHash('sha256').update(normalized).digest('hex');
const result = hashPhoneNumber('+14155552671');
expect(result).toBe(`sha256_${expectedHash}`);
});
});

View File

@@ -2,23 +2,53 @@ import { z } from 'zod';
// Environment variables for analytics // Environment variables for analytics
const envSchema = z.object({ const envSchema = z.object({
MIXPANEL_TOKEN: z.string(), MIXPANEL_TOKEN: z.string().min(1, 'MIXPANEL_TOKEN is required for analytics'),
MIXPANEL_API_SECRET: z.string().optional(), MIXPANEL_API_SECRET: z.string().optional(),
GA4_MEASUREMENT_ID: z.string(), GA4_MEASUREMENT_ID: z.string().min(1, 'GA4_MEASUREMENT_ID is required for analytics'),
GA4_API_SECRET: z.string().optional(), GA4_API_SECRET: z.string().optional(),
STRIPE_WEBHOOK_SECRET: z.string(), STRIPE_WEBHOOK_SECRET: z.string().optional(),
ANALYTICS_ENV: z.enum(['development', 'production', 'staging']).default('development'), ANALYTICS_ENV: z.enum(['development', 'production', 'staging']).default('development'),
}); });
function getEnvValue(key: string): string | undefined {
const value = process.env[key];
if (!value) {
return undefined;
}
return value;
}
const rawEnv = {
MIXPANEL_TOKEN: getEnvValue('MIXPANEL_TOKEN'),
MIXPANEL_API_SECRET: getEnvValue('MIXPANEL_API_SECRET'),
GA4_MEASUREMENT_ID: getEnvValue('GA4_MEASUREMENT_ID'),
GA4_API_SECRET: getEnvValue('GA4_API_SECRET'),
STRIPE_WEBHOOK_SECRET: getEnvValue('STRIPE_WEBHOOK_SECRET'),
ANALYTICS_ENV: getEnvValue('ANALYTICS_ENV'),
};
const missingRequired: string[] = [];
if (!rawEnv.MIXPANEL_TOKEN) missingRequired.push('MIXPANEL_TOKEN');
if (!rawEnv.GA4_MEASUREMENT_ID) missingRequired.push('GA4_MEASUREMENT_ID');
if (missingRequired.length > 0) {
console.warn(
`[Analytics] Missing required environment variables: ${missingRequired.join(', ')}. ` +
`Analytics will operate in degraded mode. Set these in your .env file.`
);
}
export const analyticsEnv = envSchema.parse({ export const analyticsEnv = envSchema.parse({
MIXPANEL_TOKEN: process.env.MIXPANEL_TOKEN, MIXPANEL_TOKEN: rawEnv.MIXPANEL_TOKEN || '__MISSING__',
MIXPANEL_API_SECRET: process.env.MIXPANEL_API_SECRET, MIXPANEL_API_SECRET: rawEnv.MIXPANEL_API_SECRET,
GA4_MEASUREMENT_ID: process.env.GA4_MEASUREMENT_ID, GA4_MEASUREMENT_ID: rawEnv.GA4_MEASUREMENT_ID || '__MISSING__',
GA4_API_SECRET: process.env.GA4_API_SECRET, GA4_API_SECRET: rawEnv.GA4_API_SECRET,
STRIPE_WEBHOOK_SECRET: process.env.STRIPE_WEBHOOK_SECRET, STRIPE_WEBHOOK_SECRET: rawEnv.STRIPE_WEBHOOK_SECRET,
ANALYTICS_ENV: process.env.ANALYTICS_ENV, ANALYTICS_ENV: rawEnv.ANALYTICS_ENV,
}); });
export const isAnalyticsConfigured = !missingRequired.length;
// Event taxonomy // Event taxonomy
export enum EventType { export enum EventType {
// User events // User events
@@ -63,15 +93,15 @@ export enum EventType {
REFERRAL_CONVERTED = 'referral_converted', REFERRAL_CONVERTED = 'referral_converted',
} }
// Event properties schema // Event properties schema - accepts common properties and allows extension
export const eventPropertiesSchema = z.object({ export const eventPropertiesSchema = z.object({
userId: z.string().optional(), userId: z.string().optional(),
sessionId: z.string().optional(), sessionId: z.string().optional(),
timestamp: z.date().optional(), timestamp: z.union([z.date(), z.string().datetime()]).optional(),
platform: z.enum(['web', 'mobile', 'desktop', 'api']).optional(), platform: z.enum(['web', 'mobile', 'desktop', 'api']).optional(),
version: z.string().optional(), version: z.string().optional(),
environment: z.string().optional(), environment: z.string().optional(),
}); }).passthrough();
// KPI definitions // KPI definitions
export const kpiDefinitions = { export const kpiDefinitions = {

View File

@@ -5,6 +5,7 @@ export {
eventPropertiesSchema, eventPropertiesSchema,
kpiDefinitions, kpiDefinitions,
alertThresholds, alertThresholds,
isAnalyticsConfigured,
} from './config/analytics.config'; } from './config/analytics.config';
// Services // Services
@@ -16,3 +17,8 @@ export {
GA4Service, GA4Service,
ga4Service, ga4Service,
} from './services/ga4.service'; } from './services/ga4.service';
// Utils
export {
hashPhoneNumber,
} from './utils/phone-hash';

View File

@@ -4,14 +4,22 @@ import { hashPhoneNumber } from '../utils/phone-hash';
// Mixpanel service // Mixpanel service
export class MixpanelService { export class MixpanelService {
private static _instance: MixpanelService | null = null;
private client: Analytics; private client: Analytics;
constructor() { private constructor() {
this.client = new Analytics({ this.client = new Analytics({
apiKey: analyticsEnv.MIXPANEL_TOKEN, apiKey: analyticsEnv.MIXPANEL_TOKEN,
}); });
} }
public static getInstance(): MixpanelService {
if (!MixpanelService._instance) {
MixpanelService._instance = new MixpanelService();
}
return MixpanelService._instance;
}
/** /**
* Track an event in Mixpanel * Track an event in Mixpanel
*/ */
@@ -26,8 +34,9 @@ export class MixpanelService {
event, event,
distinctId, distinctId,
properties: { properties: {
...validatedProperties,
...properties, ...properties,
...validatedProperties,
timestamp: new Date(),
}, },
}); });
} }
@@ -113,5 +122,5 @@ export class MixpanelService {
} }
} }
// Export instance // Export singleton instance
export const mixpanelService = new MixpanelService(); export const mixpanelService = MixpanelService.getInstance();

View File

@@ -1,10 +1,36 @@
import crypto from 'crypto'; import crypto from 'crypto';
/** /**
* Hash a phone number for analytics purposes * Normalize phone number to E.164 format before hashing.
* Uses SHA-256 for consistent, cryptographically strong hashing * Strips all non-digit characters, handles common formats.
* Ensures consistent hashing regardless of input format.
*/
function normalizePhoneNumber(phoneNumber: string): string {
const digits = phoneNumber.replace(/\D/g, '');
if (digits.length === 11 && digits.startsWith('1')) {
return `+${digits}`;
}
if (digits.length === 10) {
return `+1${digits}`;
}
if (digits.length > 10 && !digits.startsWith('1')) {
return `+${digits}`;
}
return `+${digits}`;
}
/**
* Hash a phone number for analytics purposes.
* Normalizes to E.164 before hashing so different formats
* (+1-415-555-2671, 4155552671, +14155552671) produce the same hash.
* Uses SHA-256 for consistent, cryptographically strong hashing.
*/ */
export function hashPhoneNumber(phoneNumber: string): string { export function hashPhoneNumber(phoneNumber: string): string {
const hash = crypto.createHash('sha256').update(phoneNumber).digest('hex'); const normalized = normalizePhoneNumber(phoneNumber);
const hash = crypto.createHash('sha256').update(normalized).digest('hex');
return `sha256_${hash}`; return `sha256_${hash}`;
} }

View File

@@ -12,6 +12,7 @@
"typecheck": "tsc --noEmit" "typecheck": "tsc --noEmit"
}, },
"dependencies": { "dependencies": {
"@shieldsai/shared-analytics": "workspace:*",
"@shieldai/db": "workspace:*", "@shieldai/db": "workspace:*",
"@shieldai/types": "workspace:*", "@shieldai/types": "workspace:*",
"@shieldai/correlation": "workspace:*", "@shieldai/correlation": "workspace:*",

View File

@@ -1,3 +1,4 @@
import { mixpanelService, EventType } from '@shieldsai/shared-analytics';
import { FieldEncryptionService } from '@shieldai/db'; import { FieldEncryptionService } from '@shieldai/db';
export interface SpamBlockedEvent { export interface SpamBlockedEvent {
@@ -30,6 +31,14 @@ const DEFAULT_CONFIG: Required<MixpanelConfig> = {
enableLogging: true, enableLogging: true,
}; };
/**
* SpamShield analytics adapter.
* Delegates to the shared MixpanelService for consistent event tracking
* across the ShieldAI platform, while maintaining spam-specific interfaces.
*
* @deprecated Use {@link @shieldsai/shared-analytics#MixpanelService} directly
* for new analytics code. This wrapper maintains backward compatibility.
*/
export class MixpanelService { export class MixpanelService {
private readonly config: Required<MixpanelConfig>; private readonly config: Required<MixpanelConfig>;
private readonly events: MixpanelEventProperties[] = []; private readonly events: MixpanelEventProperties[] = [];
@@ -58,15 +67,24 @@ export class MixpanelService {
); );
} }
const response = await this.track('spam_blocked', properties); await mixpanelService.track(EventType.SPAM_BLOCKED, properties.phoneNumberHash, {
decision: event.decision,
confidence: event.confidence,
ruleMatches: event.ruleMatches,
timestamp: event.timestamp,
});
return { return properties;
...properties,
...response,
};
} }
async track(eventName: string, properties: Record<string, any>): Promise<Record<string, any>> { async track(eventName: string, properties: Record<string, any>): Promise<Record<string, any>> {
const mpEvent = Object.values(EventType).find(e => e === eventName) as EventType | undefined;
if (mpEvent) {
await mixpanelService.track(mpEvent, properties.phoneNumberHash || 'anonymous', properties);
return { status: 200 };
}
const url = `https://${this.config.apiHost}/track`; const url = `https://${this.config.apiHost}/track`;
const payload = { const payload = {