Mike/FrenoCorp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix FRE-622 security findings: IDOR, auth, markdown injection, email validation

Browse Source 
H-1: Add createdBy to alertRules, IDOR check on update/delete
H-2: Add createdBy to scheduledReports, IDOR check on update
H-3: Add createdBy to cohorts, IDOR check on addCohortMember
M-1: Change submitNPSResponse to protectedProcedure
M-2: Escape Slack Markdown special chars in alert rule names
M-3: Change getAllLatestKPIs, getAlertRules, getAlerts, getNPSResponses to protectedProcedure
L-2: Add email regex validation to recipients field

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Michael Freno
2026-04-29 00:28:01 -04:00
parent ed83f29fe6
commit eab380b76b
6 changed files with 80 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
server/trpc/analytics-router.ts
View File

@@ -88,7 +88,7 @@ export const analyticsRouter = {
return await getLatestKPI(ctx.db!, input.kpiKey as KPIKey);
}),
getAllLatestKPIs: publicProcedure.query(async ({ ctx }) => {
getAllLatestKPIs: protectedProcedure.query(async ({ ctx }) => {
return await getAllLatestKPIs(ctx.db!);
}),
@@ -147,7 +147,7 @@ export const analyticsRouter = {
// --- Alert Endpoints ---
getAlertRules: publicProcedure.query(async ({ ctx }) => {
getAlertRules: protectedProcedure.query(async ({ ctx }) => {
return await ctx.db!.select().from(alertRules).orderBy(desc(alertRules.createdAt));
}),
@@ -171,6 +171,7 @@ export const analyticsRouter = {
channelId: input.channelId ?? null,
isActive: true,
cooldownMinutes: input.cooldownMinutes,
createdBy: ctx.userId,
}).returning();
return result[0];
}),
@@ -188,6 +189,18 @@ export const analyticsRouter = {
}))
.mutation(async ({ input, ctx }) => {
const { id, ...updates } = input;
const existing = await ctx.db!
.select({ id: alertRules.id, createdBy: alertRules.createdBy })
.from(alertRules)
.where(eq(alertRules.id, id))
.limit(1);
const rule = existing[0];
if (!rule) {
throw new (await import('./router')).TRPCError({ code: 'NOT_FOUND', message: 'Alert rule not found' });
}
if (rule.createdBy !== ctx.userId) {
throw new (await import('./router')).TRPCError({ code: 'FORBIDDEN', message: 'Not the rule owner' });
}
const result = await ctx.db!
.update(alertRules)
.set({ ...updates, updatedAt: new Date() })
@@ -199,11 +212,23 @@ export const analyticsRouter = {
deleteAlertRule: protectedProcedure
.input(z.object({ id: z.number().int().positive() }))
.mutation(async ({ input, ctx }) => {
const existing = await ctx.db!
.select({ id: alertRules.id, createdBy: alertRules.createdBy })
.from(alertRules)
.where(eq(alertRules.id, input.id))
.limit(1);
const rule = existing[0];
if (!rule) {
throw new (await import('./router')).TRPCError({ code: 'NOT_FOUND', message: 'Alert rule not found' });
}
if (rule.createdBy !== ctx.userId) {
throw new (await import('./router')).TRPCError({ code: 'FORBIDDEN', message: 'Not the rule owner' });
}
await ctx.db!.delete(alertRules).where(eq(alertRules.id, input.id));
return { success: true };
}),
getAlerts: publicProcedure
getAlerts: protectedProcedure
.input(z.object({
severity: AlertSeveritySchema.optional(),
limit: z.number().int().min(1).max(200).default(50),
@@ -274,7 +299,7 @@ export const analyticsRouter = {
name: z.string().min(1).max(200),
reportType: ReportTypeSchema,
schedule: ScheduleSchema,
recipients: z.string(),
recipients: z.string().regex(/^[^\s@]+@[^\s@]+\.[^\s@]+(,[^\s@]+@[^\s@]+\.[^\s@]+)*$/, 'Each recipient must be a valid email'),
format: ReportFormatSchema,
metadata: z.record(z.string(), z.unknown()).optional(),
}))
@@ -287,6 +312,7 @@ export const analyticsRouter = {
format: input.format,
isActive: true,
metadata: input.metadata ? JSON.stringify(input.metadata) : null,
createdBy: ctx.userId,
});
return report;
}),
@@ -297,12 +323,24 @@ export const analyticsRouter = {
name: z.string().min(1).max(200).optional(),
reportType: ReportTypeSchema.optional(),
schedule: ScheduleSchema.optional(),
recipients: z.string().optional(),
recipients: z.string().regex(/^[^\s@]+@[^\s@]+\.[^\s@]+(,[^\s@]+@[^\s@]+\.[^\s@]+)*$/, 'Each recipient must be a valid email').optional(),
format: ReportFormatSchema.optional(),
isActive: z.boolean().optional(),
}))
.mutation(async ({ input, ctx }) => {
const { id, ...updates } = input;
const existing = await ctx.db!
.select({ id: scheduledReports.id, createdBy: scheduledReports.createdBy })
.from(scheduledReports)
.where(eq(scheduledReports.id, id))
.limit(1);
const report = existing[0];
if (!report) {
throw new (await import('./router')).TRPCError({ code: 'NOT_FOUND', message: 'Scheduled report not found' });
}
if (report.createdBy !== ctx.userId) {
throw new (await import('./router')).TRPCError({ code: 'FORBIDDEN', message: 'Not the report owner' });
}
const result = await ctx.db!
.update(scheduledReports)
.set({ ...updates, updatedAt: new Date() })
@@ -341,6 +379,7 @@ export const analyticsRouter = {
periodStart: new Date(input.periodStart),
periodEnd: input.periodEnd ? new Date(input.periodEnd) : undefined,
filterCriteria: input.filterCriteria,
createdBy: ctx.userId,
});
return cohort;
}),
@@ -351,6 +390,18 @@ export const analyticsRouter = {
userId: z.number().int().positive(),
}))
.mutation(async ({ input, ctx }) => {
const existing = await ctx.db!
.select({ id: cohorts.id, createdBy: cohorts.createdBy })
.from(cohorts)
.where(eq(cohorts.id, input.cohortId))
.limit(1);
const cohort = existing[0];
if (!cohort) {
throw new (await import('./router')).TRPCError({ code: 'NOT_FOUND', message: 'Cohort not found' });
}
if (cohort.createdBy !== ctx.userId) {
throw new (await import('./router')).TRPCError({ code: 'FORBIDDEN', message: 'Not the cohort owner' });
}
await addCohortMember(ctx.db!, input.cohortId, input.userId);
const size = await getCohortSize(ctx.db!, input.cohortId);
return { success: true, cohortSize: size };
@@ -382,7 +433,7 @@ export const analyticsRouter = {
// --- NPS Endpoints ---
submitNPSResponse: publicProcedure
submitNPSResponse: protectedProcedure
.input(z.object({
score: z.number().int().min(0).max(10),
userId: z.number().int().positive().optional(),
@@ -414,7 +465,7 @@ export const analyticsRouter = {
);
}),
getNPSResponses: publicProcedure
getNPSResponses: protectedProcedure
.input(z.object({
category: z.enum(['detractor', 'passive', 'promoter']).optional(),
periodStart: z.string().datetime().optional(),

2
src/db/schema/alert_rules.ts
View File

@@ -1,4 +1,5 @@
import { sqliteTable, text, integer, real } from "drizzle-orm/sqlite-core";
import { users } from "./users";
export const alertRules = sqliteTable("alert_rules", {
id: integer("id").primaryKey({ autoIncrement: true }),
@@ -10,6 +11,7 @@ export const alertRules = sqliteTable("alert_rules", {
channelId: text("channel_id"),
isActive: integer("is_active", { mode: "boolean" }).notNull().default(true),
cooldownMinutes: integer("cooldown_minutes").notNull().default(60),
createdBy: integer("created_by").references(() => users.id),
createdAt: integer("created_at", { mode: "timestamp" }).notNull().default(new Date()),
updatedAt: integer("updated_at", { mode: "timestamp" }).notNull().default(new Date()),
});

1
src/db/schema/cohorts.ts
View File

@@ -10,6 +10,7 @@ export const cohorts = sqliteTable("cohorts", {
size: integer("size").notNull().default(0),
retentionData: text("retention_data"),
metadata: text("metadata"),
createdBy: integer("created_by").references(() => users.id),
createdAt: integer("created_at", { mode: "timestamp" }).notNull().default(new Date()),
updatedAt: integer("updated_at", { mode: "timestamp" }).notNull().default(new Date()),
});

2
src/db/schema/scheduled_reports.ts
View File

@@ -1,4 +1,5 @@
import { sqliteTable, text, integer } from "drizzle-orm/sqlite-core";
import { users } from "./users";
export const scheduledReports = sqliteTable("scheduled_reports", {
id: integer("id").primaryKey({ autoIncrement: true }),
@@ -11,6 +12,7 @@ export const scheduledReports = sqliteTable("scheduled_reports", {
lastRunAt: integer("last_run_at", { mode: "timestamp" }),
nextRunAt: integer("next_run_at", { mode: "timestamp" }),
metadata: text("metadata"),
createdBy: integer("created_by").references(() => users.id),
createdAt: integer("created_at", { mode: "timestamp" }).notNull().default(new Date()),
updatedAt: integer("updated_at", { mode: "timestamp" }).notNull().default(new Date()),
});

2
src/lib/analytics/cohort-analysis.ts
View File

@@ -9,6 +9,7 @@ export interface CohortDefinition {
periodStart: Date;
periodEnd?: Date;
filterCriteria: Record<string, unknown>;
createdBy?: number;
}
export interface CohortAnalysisResult {
@@ -37,6 +38,7 @@ export async function createCohort(
size: 0,
retentionData: null,
metadata: definition.description ? JSON.stringify({ description: definition.description }) : null,
createdBy: definition.createdBy ?? null,
};
const result = await db.insert(cohorts).values(cohort).returning();

16
src/lib/analytics/slack-alerts.ts
View File

@@ -108,6 +108,18 @@ function mapSeverity(
return rule.severity;
}
function escapeSlackMarkdown(text: string): string {
return text
.replace(/&/g, '&amp;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
.replace(/\*/g, '\\*')
.replace(/_/g, '\\_')
.replace(/~/g, '\\~')
.replace(/`/g, '\\`')
.replace(/\(/g, '\\(');
}
export function formatAlertMessage(
rule: typeof alertRules.$inferSelect,
value: number,
@@ -118,7 +130,9 @@ export function formatAlertMessage(
? rule.threshold.toString()
: rule.threshold.toFixed(2);
return `${rule.name}: ${kpiKey} is ${formattedValue} (${rule.condition} ${formattedThreshold})`;
const safeName = escapeSlackMarkdown(rule.name);
return `${safeName}: ${kpiKey} is ${formattedValue} (${rule.condition} ${formattedThreshold})`;
}
export async function sendSlackAlert(