fix: address code review findings for analytics and security

This commit is contained in:
syntaxbullet
2026-01-08 21:39:01 +01:00
parent 11e07a0068
commit 6763e3c543
6 changed files with 114 additions and 26 deletions

View File

@@ -13,7 +13,13 @@ import {
bigserial,
check
} from 'drizzle-orm/pg-core';
import { relations, sql } from 'drizzle-orm';
import { relations, sql, type InferSelectModel } from 'drizzle-orm';
export type User = InferSelectModel<typeof users>;
export type Transaction = InferSelectModel<typeof transactions>;
export type ModerationCase = InferSelectModel<typeof moderationCases>;
export type Item = InferSelectModel<typeof items>;
export type Inventory = InferSelectModel<typeof inventory>;
// --- TABLES ---

View File

@@ -227,5 +227,66 @@ describe("dashboardService", () => {
expect(otherHour?.transactions).toBe(0);
expect(otherHour?.commands).toBe(0);
});
test("should return 24 hours of zeros if database is empty", async () => {
mockSelect.mockImplementationOnce(() => ({
// @ts-ignore
from: mock(() => ({
where: mock(() => ({
groupBy: mock(() => ({
orderBy: mock(() => Promise.resolve([]))
}))
}))
}))
}));
const activity = await dashboardService.getActivityAggregation();
expect(activity).toHaveLength(24);
expect(activity.every(a => a.transactions === 0 && a.commands === 0)).toBe(true);
});
test("should return 24 hours of zeros if database returns rows with null hours", async () => {
mockSelect.mockImplementationOnce(() => ({
// @ts-ignore
from: mock(() => ({
where: mock(() => ({
groupBy: mock(() => ({
orderBy: mock(() => Promise.resolve([{ hour: null, transactions: "10", commands: "5" }]))
}))
}))
}))
}));
const activity = await dashboardService.getActivityAggregation();
expect(activity).toHaveLength(24);
expect(activity.every(a => a.transactions === 0 && a.commands === 0)).toBe(true);
});
test("should correctly map hours regardless of input sort order", async () => {
const now = new Date();
now.setHours(now.getHours(), 0, 0, 0);
const hourAgo = new Date(now.getTime() - 60 * 60 * 1000);
mockSelect.mockImplementationOnce(() => ({
// @ts-ignore
from: mock(() => ({
where: mock(() => ({
groupBy: mock(() => ({
orderBy: mock(() => Promise.resolve([
{ hour: now.toISOString(), transactions: "10", commands: "5" },
{ hour: hourAgo.toISOString(), transactions: "20", commands: "10" }
]))
}))
}))
}))
}));
const activity = await dashboardService.getActivityAggregation();
const current = activity.find(a => a.hour === now.toISOString());
const past = activity.find(a => a.hour === hourAgo.toISOString());
expect(current?.transactions).toBe(10);
expect(past?.transactions).toBe(20);
});
});
});

View File

@@ -1,6 +1,6 @@
import { DrizzleClient } from "@shared/db/DrizzleClient";
import { users, transactions, moderationCases, inventory } from "@db/schema";
import { desc, sql, and, gte } from "drizzle-orm";
import { users, transactions, moderationCases, inventory, type User } from "@db/schema";
import { desc, sql, gte } from "drizzle-orm";
import type { RecentEvent, ActivityData } from "./dashboard.types";
import { TransactionType } from "@shared/lib/constants";
@@ -39,18 +39,18 @@ export const dashboardService = {
const allUsers = await DrizzleClient.select().from(users);
const totalWealth = allUsers.reduce(
(acc: bigint, u: any) => acc + (u.balance || 0n),
(acc: bigint, u: User) => acc + (u.balance || 0n),
0n
);
const avgLevel = allUsers.length > 0
? Math.round(
allUsers.reduce((acc: number, u: any) => acc + (u.level || 1), 0) / allUsers.length
allUsers.reduce((acc: number, u: User) => acc + (u.level || 1), 0) / allUsers.length
)
: 1;
const topStreak = allUsers.reduce(
(max: number, u: any) => Math.max(max, u.dailyStreak || 0),
(max: number, u: User) => Math.max(max, u.dailyStreak || 0),
0
);
@@ -83,7 +83,7 @@ export const dashboardService = {
},
});
return recentTx.map((tx: any) => ({
return recentTx.map((tx) => ({
type: 'info' as const,
message: `${tx.user?.username || 'Unknown'}: ${tx.description || 'Transaction'}`,
timestamp: tx.createdAt || new Date(),
@@ -103,11 +103,11 @@ export const dashboardService = {
where: gte(moderationCases.createdAt, oneDayAgo),
});
return recentCases.map((modCase: any) => ({
return recentCases.map((modCase) => ({
type: modCase.type === 'warn' || modCase.type === 'ban' ? 'error' : 'info',
message: `${modCase.type.toUpperCase()}: ${modCase.username} - ${modCase.reason}`,
timestamp: modCase.createdAt || new Date(),
icon: getModerationIcon(modCase.type),
icon: getModerationIcon(modCase.type as string),
}));
},

View File

@@ -15,23 +15,22 @@ interface ActivityChartProps {
loading?: boolean;
}
const CustomTooltip = ({ active, payload, label }: any) => {
const CustomTooltip = ({ active, payload }: any) => {
if (active && payload && payload.length) {
const date = new Date(label);
const timeStr = date.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' });
const data = payload[0].payload;
return (
<div className="glass p-3 rounded-lg border border-white/10 text-sm shadow-xl animate-in fade-in zoom-in duration-200">
<p className="font-semibold text-white/90 border-b border-white/10 pb-1 mb-2">
{timeStr}
{data.displayTime}
</p>
<div className="space-y-1">
<p className="flex items-center justify-between gap-4">
<span className="text-blue-400">Commands</span>
<span className="text-primary font-medium">Commands</span>
<span className="font-mono">{payload[0].value}</span>
</p>
<p className="flex items-center justify-between gap-4">
<span className="text-emerald-400">Transactions</span>
<span className="text-[var(--chart-2)] font-medium">Transactions</span>
<span className="font-mono">{payload[1].value}</span>
</p>
</div>
@@ -72,8 +71,8 @@ export const ActivityChart: React.FC<ActivityChartProps> = ({ data, loading }) =
<stop offset="95%" stopColor="var(--color-primary)" stopOpacity={0} />
</linearGradient>
<linearGradient id="colorTransactions" x1="0" y1="0" x2="0" y2="1">
<stop offset="5%" stopColor="oklch(0.7 0.15 160)" stopOpacity={0.3} />
<stop offset="95%" stopColor="oklch(0.7 0.15 160)" stopOpacity={0} />
<stop offset="5%" stopColor="var(--chart-2)" stopOpacity={0.3} />
<stop offset="95%" stopColor="var(--chart-2)" stopOpacity={0} />
</linearGradient>
</defs>
<CartesianGrid
@@ -113,7 +112,7 @@ export const ActivityChart: React.FC<ActivityChartProps> = ({ data, loading }) =
<Area
type="monotone"
dataKey="transactions"
stroke="oklch(0.7 0.15 160)"
stroke="var(--chart-2)"
strokeWidth={2}
fillOpacity={1}
fill="url(#colorTransactions)"

View File

@@ -25,7 +25,12 @@ export function useActivityStats(): UseActivityStatsResult {
const fetchActivity = async () => {
setLoading(true);
try {
const response = await fetch("/api/stats/activity");
const token = (window as any).AURORA_ENV?.ADMIN_TOKEN;
const response = await fetch("/api/stats/activity", {
headers: {
"Authorization": `Bearer ${token}`
}
});
if (!response.ok) throw new Error(`HTTP error! status: ${response.status}`);
const jsonData = await response.json();
setData(jsonData);

View File

@@ -60,7 +60,8 @@ export async function createWebServer(config: WebServerConfig = {}): Promise<Web
let statsBroadcastInterval: Timer | undefined;
// Cache for activity stats (heavy aggregation)
let cachedActivity: { data: any, timestamp: number } | null = null;
let activityPromise: Promise<any> | null = null;
let lastActivityFetch: number = 0;
const ACTIVITY_CACHE_TTL = 5 * 60 * 1000; // 5 minutes
const server = serve({
@@ -103,15 +104,31 @@ export async function createWebServer(config: WebServerConfig = {}): Promise<Web
if (url.pathname === "/api/stats/activity") {
try {
const now = Date.now();
if (cachedActivity && (now - cachedActivity.timestamp < ACTIVITY_CACHE_TTL)) {
return Response.json(cachedActivity.data);
// Security Check: Token-based authentication
const { env } = await import("@shared/lib/env");
const authHeader = req.headers.get("Authorization");
if (authHeader !== `Bearer ${env.ADMIN_TOKEN}`) {
return new Response("Unauthorized", { status: 401 });
}
const { dashboardService } = await import("@shared/modules/dashboard/dashboard.service");
const activity = await dashboardService.getActivityAggregation();
const now = Date.now();
cachedActivity = { data: activity, timestamp: now };
// If we have a valid cache, return it
if (activityPromise && (now - lastActivityFetch < ACTIVITY_CACHE_TTL)) {
const data = await activityPromise;
return Response.json(data);
}
// Otherwise, trigger a new fetch (deduplicated by the promise)
if (!activityPromise || (now - lastActivityFetch >= ACTIVITY_CACHE_TTL)) {
activityPromise = (async () => {
const { dashboardService } = await import("@shared/modules/dashboard/dashboard.service");
return await dashboardService.getActivityAggregation();
})();
lastActivityFetch = now;
}
const activity = await activityPromise;
return Response.json(activity);
} catch (error) {
console.error("Error fetching activity stats:", error);