From 36e9409de791cdc0a01d2aa095c434d90c8df78c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 26 Mar 2026 18:20:33 +0000 Subject: [PATCH 1/4] fix: resolve three open bugs (#278, #280, #281) - #281: Replace fragile exact call-count assertion in migration test with toBeGreaterThan(0), relying on per-DDL string assertions for coverage - #280: Add requireCampaignConfigsReady lazy-retry guard to all /api/admin/automated-campaigns routes, matching the existing requireConditionsReady pattern - #278: Add Zod schema validation to POST /api/admin/error-logs/batch-delete request body, rejecting invalid types and unexpected properties with 400 https://claude.ai/code/session_01QqzNhpF5Qy7W8vrxiFfmgD --- server/routes.deleteErrorLog.test.ts | 23 +++++----- server/routes.migration.test.ts | 14 ++---- server/routes.ts | 66 ++++++++++++++++++++-------- 3 files changed, 61 insertions(+), 42 deletions(-) diff --git a/server/routes.deleteErrorLog.test.ts b/server/routes.deleteErrorLog.test.ts index 9f51132..62f3ab5 100644 --- a/server/routes.deleteErrorLog.test.ts +++ b/server/routes.deleteErrorLog.test.ts @@ -521,7 +521,7 @@ describe("POST /api/admin/error-logs/batch-delete", () => { const req = { user: { claims: { sub: "owner-123" } }, body: {} }; const res = await callHandler("post", ENDPOINT, req); expect(res._status).toBe(400); - expect(res._json).toEqual({ message: "Must provide ids or filters" }); + expect(res._json.message).toBe("Invalid request body"); }); it("returns 400 when both ids and filters provided", async () => { @@ -529,7 +529,7 @@ describe("POST /api/admin/error-logs/batch-delete", () => { const req = { user: { claims: { sub: "owner-123" } }, body: { ids: [1], filters: { level: "error" } } }; const res = await callHandler("post", ENDPOINT, req); expect(res._status).toBe(400); - expect(res._json).toEqual({ message: "Provide either ids or filters, not both" }); + expect(res._json.message).toBe("Invalid request body"); }); it("returns 400 for empty ids array", async () => { @@ -538,7 +538,7 @@ describe("POST /api/admin/error-logs/batch-delete", () => { const req = { user: { claims: { sub: "owner-123" } }, body: { ids: [] } }; const res = await callHandler("post", ENDPOINT, req); expect(res._status).toBe(400); - expect(res._json).toEqual({ message: "ids must be a non-empty array of positive integers" }); + expect(res._json.message).toBe("Invalid request body"); }); it("returns 400 for ids containing non-integers", async () => { @@ -547,7 +547,7 @@ describe("POST /api/admin/error-logs/batch-delete", () => { const req = { user: { claims: { sub: "owner-123" } }, body: { ids: [1, "abc", 3] } }; const res = await callHandler("post", ENDPOINT, req); expect(res._status).toBe(400); - expect(res._json).toEqual({ message: "ids must be a non-empty array of positive integers" }); + expect(res._json.message).toBe("Invalid request body"); }); it("returns 400 for ids containing zero or negative values", async () => { @@ -556,7 +556,7 @@ describe("POST /api/admin/error-logs/batch-delete", () => { const req = { user: { claims: { sub: "owner-123" } }, body: { ids: [0, -1] } }; const res = await callHandler("post", ENDPOINT, req); expect(res._status).toBe(400); - expect(res._json).toEqual({ message: "ids must be a non-empty array of positive integers" }); + expect(res._json.message).toBe("Invalid request body"); }); it("soft-deletes authorized entries by IDs for app owner", async () => { @@ -671,7 +671,7 @@ describe("POST /api/admin/error-logs/batch-delete", () => { }; const res = await callHandler("post", ENDPOINT, req); expect(res._status).toBe(400); - expect(res._json).toEqual({ message: "filters must include at least one of: level, source" }); + expect(res._json.message).toBe("Invalid request body"); }); it("rejects empty filters even with excludeIds", async () => { @@ -684,23 +684,20 @@ describe("POST /api/admin/error-logs/batch-delete", () => { }; const res = await callHandler("post", ENDPOINT, req); expect(res._status).toBe(400); - expect(res._json).toEqual({ message: "filters must include at least one of: level, source" }); + expect(res._json.message).toBe("Invalid request body"); }); - it("ignores non-integer excludeIds with valid filters", async () => { + it("rejects non-integer excludeIds with valid filters", async () => { mockGetUser.mockResolvedValue({ tier: "power" }); mockGetMonitors.mockResolvedValue([]); - mockSelectWhereFn.mockResolvedValue([ - { id: 1, context: null }, - ]); const req = { user: { claims: { sub: "owner-123" } }, body: { filters: { level: "error" }, excludeIds: ["abc", -1, 0] }, }; const res = await callHandler("post", ENDPOINT, req); - expect(res._status).toBe(200); - expect(res._json.count).toBe(1); + expect(res._status).toBe(400); + expect(res._json.message).toBe("Invalid request body"); }); it("returns 500 when database throws", async () => { diff --git a/server/routes.migration.test.ts b/server/routes.migration.test.ts index 99c785f..e59b6fd 100644 --- a/server/routes.migration.test.ts +++ b/server/routes.migration.test.ts @@ -167,16 +167,10 @@ describe("error_logs dedup column migration at startup", () => { const app = makeMockApp(); await registerRoutes(app as any, app as any); - // db.execute should have been called 25 times: - // 2 for the ALTER TABLE monitors statements (health_alert_sent_at, last_healthy_at) - // 3 for the ALTER TABLE error_logs statements (first_occurrence, occurrence_count, deleted_at) - // 2 for the api_keys table creation (CREATE TABLE + CREATE INDEX) - // 7 for notification channel tables (3 CREATE TABLE + 3 indexes + 1 unique index) - // 3 for notification_queue columns (2 ALTER TABLE + 1 CREATE INDEX) - // 5 for tag tables (2 CREATE TABLE + 2 indexes + 1 unique index) - // 2 for monitor_conditions table (1 CREATE TABLE + 1 CREATE INDEX) - // 1 for automated_campaign_configs table (1 CREATE TABLE) - expect(mockDbExecute).toHaveBeenCalledTimes(25); + // Verify db.execute was called at least once (exact count is validated + // implicitly by the per-DDL assertions below, so we avoid a brittle + // exact-count check that breaks every time a new ensure* function is added). + expect(mockDbExecute.mock.calls.length).toBeGreaterThan(0); // Verify specific DDL statements were issued (drizzle sql`` produces SQL objects) const callStrings = mockDbExecute.mock.calls.map((c: any[]) => { diff --git a/server/routes.ts b/server/routes.ts index 36fafb4..e57388e 100644 --- a/server/routes.ts +++ b/server/routes.ts @@ -3,7 +3,7 @@ import { getResendClient } from "./services/resendClient"; import type { Express, Request, Response, NextFunction } from "express"; import { createServer, type Server } from "http"; import { storage } from "./storage"; -import { api, channelTypeSchema, webhookConfigInputSchema, slackConfigInputSchema, createTagSchema, updateTagSchema, setMonitorTagsSchema, createConditionSchema, ERROR_LOG_SOURCES } from "@shared/routes"; +import { api, channelTypeSchema, webhookConfigInputSchema, slackConfigInputSchema, createTagSchema, updateTagSchema, setMonitorTagsSchema, createConditionSchema, ERROR_LOG_SOURCES, errorLogSourceSchema } from "@shared/routes"; import { isSafeRegex } from "./services/conditions"; import { z } from "zod"; import { setupAuth, registerAuthRoutes, isAuthenticated } from "./replit_integrations/auth"; @@ -82,7 +82,7 @@ export async function registerRoutes( console.error("CRITICAL: notification_queue columns missing — notification cron queries will fail"); } await ensureTagTables(); - const campaignConfigsReady = await ensureAutomatedCampaignConfigsTable(); + let campaignConfigsReady = await ensureAutomatedCampaignConfigsTable(); if (!campaignConfigsReady) { console.error("CRITICAL: automated_campaign_configs table missing — campaign bootstrap and admin routes will fail"); } @@ -110,6 +110,27 @@ export async function registerRoutes( return true; } + // Lazy retry guard for automated campaign configs table (mirrors requireConditionsReady) + let campaignConfigsReadyProbe: Promise | null = null; + async function requireCampaignConfigsReady(res: any): Promise { + if (!campaignConfigsReady) { + campaignConfigsReadyProbe ??= ensureAutomatedCampaignConfigsTable() + .then((ready) => { + if (ready) campaignConfigsReady = true; + return campaignConfigsReady; + }) + .finally(() => { + campaignConfigsReadyProbe = null; + }); + await campaignConfigsReadyProbe; + } + if (!campaignConfigsReady) { + res.status(503).json({ message: "Campaign configs not available", code: "SERVICE_UNAVAILABLE" }); + return false; + } + return true; + } + // Setup Auth (must be before rate limiter so req.user is populated) await setupAuth(app); @@ -1533,13 +1554,23 @@ export async function registerRoutes( return res.status(403).json({ message: "Admin access required" }); } - const { ids, filters, excludeIds } = req.body; - if (!ids && !filters) { - return res.status(400).json({ message: "Must provide ids or filters" }); - } - if (ids && filters) { - return res.status(400).json({ message: "Provide either ids or filters, not both" }); + const batchDeleteSchema = z.object({ + ids: z.array(z.number().int().positive()).min(1).optional(), + filters: z.object({ + level: z.enum(["error", "warning", "info"]).optional(), + source: errorLogSourceSchema.optional(), + }).optional(), + excludeIds: z.array(z.number().int().positive()).optional(), + }).strict().refine( + (data) => (data.ids !== undefined) !== (data.filters !== undefined), + { message: "Provide either ids or filters, not both" }, + ); + + const parsed = batchDeleteSchema.safeParse(req.body); + if (!parsed.success) { + return res.status(400).json({ message: "Invalid request body", errors: parsed.error.flatten() }); } + const { ids, filters, excludeIds } = parsed.data; const isAppOwner = userId === APP_OWNER_ID; const userMonitorIds = new Set( @@ -1549,10 +1580,6 @@ export async function registerRoutes( const now = new Date(); if (ids) { - if (!Array.isArray(ids) || ids.length === 0 || !ids.every((id: any) => Number.isInteger(id) && id > 0)) { - return res.status(400).json({ message: "ids must be a non-empty array of positive integers" }); - } - const entries = await db.select().from(errorLogs) .where(and(inArray(errorLogs.id, ids), isNull(errorLogs.deletedAt))); @@ -1568,21 +1595,19 @@ export async function registerRoutes( await db.update(errorLogs).set({ deletedAt: now }).where(inArray(errorLogs.id, authorizedIds)); } res.json({ message: `${authorized.length} entries deleted`, count: authorized.length }); - } else { - const hasLevel = filters.level && ["error", "warning", "info"].includes(filters.level); - const hasSource = filters.source && (ERROR_LOG_SOURCES as readonly string[]).includes(filters.source); - if (!hasLevel && !hasSource) { + } else if (filters) { + if (!filters.level && !filters.source) { return res.status(400).json({ message: "filters must include at least one of: level, source" }); } const conditions = [isNull(errorLogs.deletedAt)]; - if (hasLevel) { + if (filters.level) { conditions.push(eq(errorLogs.level, filters.level)); } - if (hasSource) { + if (filters.source) { conditions.push(eq(errorLogs.source, filters.source)); } - const excludeList = Array.isArray(excludeIds) ? excludeIds.filter((id: any) => Number.isInteger(id) && id > 0) : []; + const excludeList = excludeIds ?? []; if (excludeList.length > 0) { conditions.push(notInArray(errorLogs.id, excludeList)); } @@ -2416,6 +2441,7 @@ export async function registerRoutes( // GET /api/admin/automated-campaigns — list all configs app.get("/api/admin/automated-campaigns", isAuthenticated, async (req: any, res) => { + if (!(await requireCampaignConfigsReady(res))) return; try { const userId = req.user?.claims?.sub; if (!userId) return res.status(401).json({ message: "Unauthorized" }); @@ -2437,6 +2463,7 @@ export async function registerRoutes( // PATCH /api/admin/automated-campaigns/:key — update config app.patch("/api/admin/automated-campaigns/:key", isAuthenticated, async (req: any, res) => { + if (!(await requireCampaignConfigsReady(res))) return; try { const userId = req.user?.claims?.sub; if (!userId) return res.status(401).json({ message: "Unauthorized" }); @@ -2485,6 +2512,7 @@ export async function registerRoutes( message: { message: "Too many trigger requests. Try again later." }, }); app.post("/api/admin/automated-campaigns/:key/trigger", isAuthenticated, autoCampaignTriggerRateLimiter, async (req: any, res) => { + if (!(await requireCampaignConfigsReady(res))) return; try { const userId = req.user?.claims?.sub; if (!userId) return res.status(401).json({ message: "Unauthorized" }); From 912133b58c89075834e5e570ebdd685e4865d478 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 26 Mar 2026 18:37:11 +0000 Subject: [PATCH 2/4] test: add Zod strict validation tests for batch-delete endpoint Add tests verifying that extra top-level and nested properties are rejected by the strict Zod schema. Also apply .strict() to the nested filters object for consistent validation. https://claude.ai/code/session_01QqzNhpF5Qy7W8vrxiFfmgD --- server/routes.deleteErrorLog.test.ts | 25 +++++++++++++++++++++++++ server/routes.ts | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/server/routes.deleteErrorLog.test.ts b/server/routes.deleteErrorLog.test.ts index 62f3ab5..ef8f41b 100644 --- a/server/routes.deleteErrorLog.test.ts +++ b/server/routes.deleteErrorLog.test.ts @@ -700,6 +700,31 @@ describe("POST /api/admin/error-logs/batch-delete", () => { expect(res._json.message).toBe("Invalid request body"); }); + it("rejects unexpected properties via strict schema", async () => { + mockGetUser.mockResolvedValue({ tier: "power" }); + + const req = { + user: { claims: { sub: "owner-123" } }, + body: { ids: [1], extraProp: "should not be here" }, + }; + const res = await callHandler("post", ENDPOINT, req); + expect(res._status).toBe(400); + expect(res._json.message).toBe("Invalid request body"); + expect(res._json.errors).toBeDefined(); + }); + + it("rejects filters with unexpected nested properties via strict schema", async () => { + mockGetUser.mockResolvedValue({ tier: "power" }); + + const req = { + user: { claims: { sub: "owner-123" } }, + body: { filters: { level: "error", unknownField: true } }, + }; + const res = await callHandler("post", ENDPOINT, req); + expect(res._status).toBe(400); + expect(res._json.message).toBe("Invalid request body"); + }); + it("returns 500 when database throws", async () => { const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); mockGetUser.mockResolvedValue({ tier: "power" }); diff --git a/server/routes.ts b/server/routes.ts index e57388e..5acd0fb 100644 --- a/server/routes.ts +++ b/server/routes.ts @@ -1559,7 +1559,7 @@ export async function registerRoutes( filters: z.object({ level: z.enum(["error", "warning", "info"]).optional(), source: errorLogSourceSchema.optional(), - }).optional(), + }).strict().optional(), excludeIds: z.array(z.number().int().positive()).optional(), }).strict().refine( (data) => (data.ids !== undefined) !== (data.filters !== undefined), From aa42f69888ee42d3048447c088184329c9483da2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 26 Mar 2026 18:40:18 +0000 Subject: [PATCH 3/4] fix: cap ids and excludeIds arrays at 500 in batch-delete schema Prevent oversized SQL IN clauses from unbounded array inputs. https://claude.ai/code/session_01QqzNhpF5Qy7W8vrxiFfmgD --- server/routes.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/routes.ts b/server/routes.ts index 5acd0fb..3e3983f 100644 --- a/server/routes.ts +++ b/server/routes.ts @@ -1555,12 +1555,12 @@ export async function registerRoutes( } const batchDeleteSchema = z.object({ - ids: z.array(z.number().int().positive()).min(1).optional(), + ids: z.array(z.number().int().positive()).min(1).max(500).optional(), filters: z.object({ level: z.enum(["error", "warning", "info"]).optional(), source: errorLogSourceSchema.optional(), }).strict().optional(), - excludeIds: z.array(z.number().int().positive()).optional(), + excludeIds: z.array(z.number().int().positive()).max(500).optional(), }).strict().refine( (data) => (data.ids !== undefined) !== (data.filters !== undefined), { message: "Provide either ids or filters, not both" }, From 8aa3d10aa449ef4bb7c42ea5341aa9ddb231d9ba Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 26 Mar 2026 18:46:50 +0000 Subject: [PATCH 4/4] fix: address skeptic findings in batch-delete validation - Reject excludeIds when combined with ids (was silently ignored) - Move empty-filters check into Zod refine for consistent error format - Add test for excludeIds + ids rejection https://claude.ai/code/session_01QqzNhpF5Qy7W8vrxiFfmgD --- server/routes.deleteErrorLog.test.ts | 11 ++++++++++- server/routes.ts | 12 +++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/server/routes.deleteErrorLog.test.ts b/server/routes.deleteErrorLog.test.ts index ef8f41b..d430482 100644 --- a/server/routes.deleteErrorLog.test.ts +++ b/server/routes.deleteErrorLog.test.ts @@ -644,7 +644,16 @@ describe("POST /api/admin/error-logs/batch-delete", () => { const req = { user: { claims: { sub: "not-the-owner" } }, body: { filters: {} } }; const res = await callHandler("post", ENDPOINT, req); expect(res._status).toBe(400); - expect(res._json).toEqual({ message: "filters must include at least one of: level, source" }); + expect(res._json.message).toBe("Invalid request body"); + }); + + it("rejects excludeIds when combined with ids", async () => { + mockGetUser.mockResolvedValue({ tier: "power" }); + + const req = { user: { claims: { sub: "owner-123" } }, body: { ids: [1, 2, 3], excludeIds: [2] } }; + const res = await callHandler("post", ENDPOINT, req); + expect(res._status).toBe(400); + expect(res._json.message).toBe("Invalid request body"); }); it("applies ownership filtering with filters mode", async () => { diff --git a/server/routes.ts b/server/routes.ts index 3e3983f..12d3569 100644 --- a/server/routes.ts +++ b/server/routes.ts @@ -1559,11 +1559,17 @@ export async function registerRoutes( filters: z.object({ level: z.enum(["error", "warning", "info"]).optional(), source: errorLogSourceSchema.optional(), - }).strict().optional(), + }).strict().refine( + (data) => data.level !== undefined || data.source !== undefined, + { message: "filters must include at least one of: level, source" }, + ).optional(), excludeIds: z.array(z.number().int().positive()).max(500).optional(), }).strict().refine( (data) => (data.ids !== undefined) !== (data.filters !== undefined), { message: "Provide either ids or filters, not both" }, + ).refine( + (data) => !(data.ids && data.excludeIds), + { message: "excludeIds cannot be used with ids" }, ); const parsed = batchDeleteSchema.safeParse(req.body); @@ -1596,10 +1602,6 @@ export async function registerRoutes( } res.json({ message: `${authorized.length} entries deleted`, count: authorized.length }); } else if (filters) { - if (!filters.level && !filters.source) { - return res.status(400).json({ message: "filters must include at least one of: level, source" }); - } - const conditions = [isNull(errorLogs.deletedAt)]; if (filters.level) { conditions.push(eq(errorLogs.level, filters.level));