From 8253de9f734b74c6756b1c50d33fe3cbcace4163 Mon Sep 17 00:00:00 2001 From: syntaxbullet Date: Thu, 8 Jan 2026 21:08:47 +0100 Subject: [PATCH] fix(dash): address safety constraints, validation, and test quality issues --- .../dashboard/dashboard.service.test.ts | 23 ++-- shared/modules/dashboard/dashboard.types.ts | 114 ++++++++++-------- web/src/server.test.ts | 75 ++++++++++++ web/src/server.ts | 43 +++++-- 4 files changed, 191 insertions(+), 64 deletions(-) create mode 100644 web/src/server.test.ts diff --git a/shared/modules/dashboard/dashboard.service.test.ts b/shared/modules/dashboard/dashboard.service.test.ts index 25aaf00..600a799 100644 --- a/shared/modules/dashboard/dashboard.service.test.ts +++ b/shared/modules/dashboard/dashboard.service.test.ts @@ -148,15 +148,15 @@ describe("dashboardService", () => { expect(events).toHaveLength(2); // Should be sorted by timestamp (newest first) - expect(events[0]?.timestamp.getTime()).toBeGreaterThanOrEqual( - events[1]?.timestamp.getTime() ?? 0 - ); + const t0 = events[0]?.timestamp instanceof Date ? events[0].timestamp.getTime() : new Date(events[0]?.timestamp ?? 0).getTime(); + const t1 = events[1]?.timestamp instanceof Date ? events[1].timestamp.getTime() : new Date(events[1]?.timestamp ?? 0).getTime(); + expect(t0).toBeGreaterThanOrEqual(t1); }); }); describe("recordEvent", () => { test("should emit NEW_EVENT to systemEvents", async () => { - const mockEmit = mock(() => { }); + const mockEmit = mock((_event: string, _data: any) => { }); mock.module("@shared/lib/events", () => ({ systemEvents: { @@ -176,10 +176,17 @@ describe("dashboardService", () => { }); expect(mockEmit).toHaveBeenCalled(); - const [eventName, data] = mockEmit.mock.calls[0] as any; - expect(eventName).toBe("dashboard:new_event"); - expect(data.message).toBe("Test Event"); - expect(data.timestamp).toBeDefined(); + const calls = mockEmit.mock.calls; + if (calls.length > 0 && calls[0]) { + expect(calls[0][0]).toBe("dashboard:new_event"); + const data = calls[0][1] as { message: string, timestamp: string }; + expect(data.message).toBe("Test Event"); + expect(data.timestamp).toBeDefined(); + // Verify it's an ISO string + expect(() => new Date(data.timestamp).toISOString()).not.toThrow(); + } else { + throw new Error("mockEmit was not called with expected arguments"); + } }); }); }); diff --git a/shared/modules/dashboard/dashboard.types.ts b/shared/modules/dashboard/dashboard.types.ts index da230c0..f312bbb 100644 --- a/shared/modules/dashboard/dashboard.types.ts +++ b/shared/modules/dashboard/dashboard.types.ts @@ -1,49 +1,69 @@ -export interface DashboardStats { - bot: { - name: string; - avatarUrl: string | null; - }; - guilds: { - count: number; - changeFromLastMonth?: number; - }; - users: { - active: number; - total: number; - changePercentFromLastMonth?: number; - }; - commands: { - total: number; - changePercentFromLastMonth?: number; - }; - ping: { - avg: number; - changeFromLastHour?: number; - }; - economy: { - totalWealth: string; // bigint as string for JSON - avgLevel: number; - topStreak: number; - }; - recentEvents: RecentEvent[]; -} +import { z } from "zod"; -export interface RecentEvent { - type: 'success' | 'error' | 'info'; - message: string; - timestamp: Date; - icon?: string; -} +export const RecentEventSchema = z.object({ + type: z.enum(['success', 'error', 'info']), + message: z.string(), + timestamp: z.union([z.date(), z.string().datetime()]), + icon: z.string().optional(), +}); -export interface ClientStats { - bot: { - name: string; - avatarUrl: string | null; - }; - guilds: number; - ping: number; - cachedUsers: number; - commandsRegistered: number; - uptime: number; - lastCommandTimestamp: number | null; -} +export type RecentEvent = z.infer; + +export const DashboardStatsSchema = z.object({ + bot: z.object({ + name: z.string(), + avatarUrl: z.string().nullable(), + }), + guilds: z.object({ + count: z.number(), + changeFromLastMonth: z.number().optional(), + }), + users: z.object({ + active: z.number(), + total: z.number(), + changePercentFromLastMonth: z.number().optional(), + }), + commands: z.object({ + total: z.number(), + changePercentFromLastMonth: z.number().optional(), + }), + ping: z.object({ + avg: z.number(), + changeFromLastHour: z.number().optional(), + }), + economy: z.object({ + totalWealth: z.string(), + avgLevel: z.number(), + topStreak: z.number(), + }), + recentEvents: z.array(RecentEventSchema), + uptime: z.number(), + lastCommandTimestamp: z.number().nullable(), +}); + +export type DashboardStats = z.infer; + +export const ClientStatsSchema = z.object({ + bot: z.object({ + name: z.string(), + avatarUrl: z.string().nullable(), + }), + guilds: z.number(), + ping: z.number(), + cachedUsers: z.number(), + commandsRegistered: z.number(), + uptime: z.number(), + lastCommandTimestamp: z.number().nullable(), +}); + +export type ClientStats = z.infer; + +// WebSocket Message Schemas +export const WsMessageSchema = z.discriminatedUnion("type", [ + z.object({ type: z.literal("PING") }), + z.object({ type: z.literal("PONG") }), + z.object({ type: z.literal("STATS_UPDATE"), data: DashboardStatsSchema }), + z.object({ type: z.literal("NEW_EVENT"), data: RecentEventSchema }), +]); + +export type WsMessage = z.infer; diff --git a/web/src/server.test.ts b/web/src/server.test.ts new file mode 100644 index 0000000..f376019 --- /dev/null +++ b/web/src/server.test.ts @@ -0,0 +1,75 @@ +import { describe, test, expect, afterAll, mock } from "bun:test"; + +// Mock the services directly to stay focused on server limits +mock.module("@shared/modules/dashboard/dashboard.service", () => ({ + dashboardService: { + getActiveUserCount: mock(() => Promise.resolve(5)), + getTotalUserCount: mock(() => Promise.resolve(10)), + getEconomyStats: mock(() => Promise.resolve({ totalWealth: 1000n, avgLevel: 5, topStreak: 2 })), + getRecentEvents: mock(() => Promise.resolve([])), + } +})); + +mock.module("../../bot/lib/clientStats", () => ({ + getClientStats: mock(() => ({ + bot: { name: "TestBot", avatarUrl: null }, + guilds: 5, + ping: 42, + cachedUsers: 100, + commandsRegistered: 10, + uptime: 3600, + lastCommandTimestamp: Date.now(), + })), +})); + +// Ensure @shared/lib/events is mocked if needed +mock.module("@shared/lib/events", () => ({ + systemEvents: { on: mock(() => { }) }, + EVENTS: { DASHBOARD: { NEW_EVENT: "dashboard:new_event" } } +})); + +import { createWebServer } from "./server"; + +describe("WebServer Security & Limits", () => { + const port = 3001; + let serverInstance: any; + + afterAll(async () => { + if (serverInstance) { + await serverInstance.stop(); + } + }); + + test("should reject more than 10 concurrent WebSocket connections", async () => { + serverInstance = await createWebServer({ port, hostname: "localhost" }); + const wsUrl = `ws://localhost:${port}/ws`; + const sockets: WebSocket[] = []; + + try { + // Attempt to open 12 connections (limit is 10) + for (let i = 0; i < 12; i++) { + const ws = new WebSocket(wsUrl); + sockets.push(ws); + await new Promise(resolve => setTimeout(resolve, 10)); + } + + // Give connections time to settle + await new Promise(resolve => setTimeout(resolve, 500)); + + // Should be exactly 10 or less + expect(serverInstance.server.pendingWebSockets).toBeLessThanOrEqual(10); + } finally { + sockets.forEach(s => s.close()); + } + }); + + test("should return 200 for health check", async () => { + if (!serverInstance) { + serverInstance = await createWebServer({ port, hostname: "localhost" }); + } + const response = await fetch(`http://localhost:${port}/api/health`); + expect(response.status).toBe(200); + const data = await response.json(); + expect(data.status).toBe("ok"); + }); +}); diff --git a/web/src/server.ts b/web/src/server.ts index 05a1af1..aa0ed6c 100644 --- a/web/src/server.ts +++ b/web/src/server.ts @@ -51,6 +51,11 @@ export async function createWebServer(config: WebServerConfig = {}): Promise= MAX_CONNECTIONS) { + console.warn(`⚠️ [WS] Connection rejected: limit reached (${currentConnections}/${MAX_CONNECTIONS})`); + return new Response("Connection limit reached", { status: 429 }); + } + const success = server.upgrade(req); if (success) return undefined; return new Response("WebSocket upgrade failed", { status: 400 }); @@ -137,30 +149,43 @@ export async function createWebServer(config: WebServerConfig = {}): Promise ({ ...event, - timestamp: event.timestamp.toISOString(), + timestamp: event.timestamp instanceof Date ? event.timestamp.toISOString() : event.timestamp, })), uptime: clientStats.uptime, lastCommandTimestamp: clientStats.lastCommandTimestamp,