Merge pull request #3886 from bluewave-labs/fix/proxy-id-empty-string

fix: deal with empty strings for proxyId

client/src/Validation/monitor.ts

@@ -69,7 +69,12 @@ const httpSchema = baseSchema.extend({
 	type: z.literal("http"),
 	url: urlSchema,
 	proxyMode: z.enum(ProxyModes),
-	proxyId: z.string().optional(),
+	// The form uses "" for "no proxy selected". drop it from the payload so the
+	// server never receives an empty ObjectId
+	proxyId: z
+		.string()
+		.transform((value) => (value === "" ? undefined : value))
+		.optional(),
 	method: z.enum(HttpMethods).optional().register(monitorStepRegistry, { step: 2 }),
 	ignoreTlsErrors: z.boolean().register(monitorStepRegistry, { step: 2 }),
 	useAdvancedMatching: z.boolean().register(monitorStepRegistry, { step: 2 }),

server/src/api/validation/monitorValidation.ts

@@ -16,6 +16,13 @@ import { DateRanges, SortOrders } from "@/types/query.js";
 
 const httpStatusCode = z.number().refine((code) => HttpStatusCodeSet.has(code), { message: "Must be a valid HTTP status code" });
 
+// The client form submits proxyId: "" when no proxy is selected, set it to undefined
+const proxyIdValidation = z
+	.string()
+	.optional()
+	.transform((value) => (value === "" ? undefined : value))
+	.refine((value) => value === undefined || /^[0-9a-f]{24}$/i.test(value), { message: "Invalid proxy ID" });
+
 export const getMonitorByIdParamValidation = z.object({
 	monitorId: z.string().min(1, "Monitor ID is required"),
 });
@@ -122,7 +129,7 @@ export const createMonitorBodyValidation = z
 		url: z.string().min(1, "URL is required"),
 		ignoreTlsErrors: z.boolean().default(false),
 		proxyMode: z.enum(ProxyModes).default("inherit"),
-		proxyId: z.string().optional(),
+		proxyId: proxyIdValidation,
 		useAdvancedMatching: z.boolean().default(false),
 		port: z.number().optional(),
 		isActive: z.boolean().optional(),
@@ -171,7 +178,7 @@ export const editMonitorBodyValidation = z
 		secret: z.string().optional(),
 		ignoreTlsErrors: z.boolean().optional(),
 		proxyMode: z.enum(ProxyModes).optional(),
-		proxyId: z.string().optional(),
+		proxyId: proxyIdValidation,
 		useAdvancedMatching: z.boolean().optional(),
 		jsonPath: z.union([z.string(), z.literal("")]).optional(),
 		expectedValue: z.union([z.string(), z.literal("")]).optional(),
@@ -233,7 +240,7 @@ const importedMonitorSchema = z
 		type: z.enum(MonitorTypes, "Invalid monitor type"),
 		ignoreTlsErrors: z.boolean().default(false),
 		proxyMode: z.enum(ProxyModes).default("inherit"),
-		proxyId: z.string().optional(),
+		proxyId: proxyIdValidation,
 		useAdvancedMatching: z.boolean().default(false),
 		jsonPath: z.union([z.string(), z.literal("")]).optional(),
 		expectedValue: z.union([z.string(), z.literal("")]).optional(),
@@ -306,7 +313,7 @@ export const monitorResponseSchema = z
 		statusWindowThreshold: z.number(),
 		ignoreTlsErrors: z.boolean(),
 		proxyMode: z.enum(ProxyModes),
-		proxyId: z.string().optional(),
+		proxyId: proxyIdValidation,
 		useAdvancedMatching: z.boolean(),
 		jsonPath: z.string().optional(),
 		expectedValue: z.string().optional(),

server/src/domain/monitors/monitor.repository.interface.ts

@@ -34,7 +34,7 @@ export interface IMonitorsRepository {
 	findByIds(monitorIds: string[], options?: { recentChecks?: RecentChecksMode }): Promise<Monitor[]>;
 
 	// update
-	updateById(monitorId: string, teamId: string, updates: Partial<Monitor>): Promise<Monitor>;
+	updateById(monitorId: string, teamId: string, updates: Partial<Monitor>, options?: { unsetProxyId?: boolean }): Promise<Monitor>;
 	updateByIds(monitorIds: string[], teamId: string, updates: Partial<Monitor>, excludeStatuses?: MonitorStatus[]): Promise<number>;
 	updateStatusWindowAndChecks(
 		monitorId: string,

server/src/domain/monitors/monitor.repository.mongo.ts

@@ -177,16 +177,14 @@ class MongoMonitorsRepository implements IMonitorsRepository {
 		return count;
 	};
 
-	updateById = async (monitorId: string, teamId: string, patch: Partial<Monitor>) => {
-		const updatedMonitor = await MonitorModel.findOneAndUpdate(
-			{ _id: monitorId, teamId },
-			{
-				$set: {
-					...patch,
-				},
-			},
-			{ new: true, runValidators: true }
-		);
+	updateById = async (monitorId: string, teamId: string, patch: Partial<Monitor>, options?: { unsetProxyId?: boolean }) => {
+		const fields: Partial<Monitor> = { ...patch };
+		const update: Record<string, unknown> = { $set: fields };
+		if (options?.unsetProxyId) {
+			delete fields.proxyId;
+			update.$unset = { proxyId: "" };
+		}
+		const updatedMonitor = await MonitorModel.findOneAndUpdate({ _id: monitorId, teamId }, update, { new: true, runValidators: true });
 		if (!updatedMonitor) {
 			throw new AppError({ message: `Failed to update monitor with id ${monitorId}`, status: 500 });
 		}

server/src/domain/monitors/monitor.service.ts

@@ -133,6 +133,10 @@ export class MonitorService implements IMonitorService {
 	}
 
 	createMonitor = async (teamId: string, userId: string, body: Monitor): Promise<void> => {
+		// proxyId is only needed in custom mode
+		if (body.proxyMode !== "custom") {
+			delete body.proxyId;
+		}
 		const monitor = await this.monitorsRepository.create(body, teamId, userId);
 		if (!monitor) {
 			throw new AppError({ message: "Failed to create monitor", status: 500, service: SERVICE_NAME, method: "createMonitor" });
@@ -375,7 +379,12 @@ export class MonitorService implements IMonitorService {
 	};
 
 	editMonitor = async ({ teamId, monitorId, body }: { teamId: string; monitorId: string; body: Partial<Monitor> }) => {
-		const editedMonitor = await this.monitorsRepository.updateById(monitorId, teamId, body);
+		// Moving off custom mode orphans the stored proxyId. Unset it so the stale reference doesn't block deleting that proxy
+		const unsetProxyId = body.proxyMode !== undefined && body.proxyMode !== "custom";
+		if (unsetProxyId) {
+			delete body.proxyId;
+		}
+		const editedMonitor = await this.monitorsRepository.updateById(monitorId, teamId, body, { unsetProxyId });
 		await this.scheduler.updateJob(editedMonitor);
 		return editedMonitor;
 	};

server/test/unit/services/monitorService.test.ts

@@ -124,6 +124,38 @@ describe("MonitorService", () => {
 			expect(jobQueue.addJob).toHaveBeenCalledWith(MONITOR_ID, monitor);
 		});
 
+		it("strips a stray proxyId when proxyMode is not custom", async () => {
+			const monitorsRepository = createMonitorsRepositoryMock();
+			const monitor = makeMonitor();
+			(monitorsRepository.create as jest.Mock).mockResolvedValue(monitor);
+			const { service } = createService({ monitorsRepository });
+
+			await service.createMonitor(TEAM_ID, USER_ID, {
+				...monitor,
+				proxyMode: "inherit",
+				proxyId: "64b7a1f2c9e77a001a2b3c4d",
+			} as any);
+
+			const createdBody = (monitorsRepository.create as jest.Mock).mock.calls[0][0] as Record<string, unknown>;
+			expect(createdBody.proxyId).toBeUndefined();
+		});
+
+		it("keeps proxyId when proxyMode is custom", async () => {
+			const monitorsRepository = createMonitorsRepositoryMock();
+			const monitor = makeMonitor();
+			(monitorsRepository.create as jest.Mock).mockResolvedValue(monitor);
+			const { service } = createService({ monitorsRepository });
+
+			await service.createMonitor(TEAM_ID, USER_ID, {
+				...monitor,
+				proxyMode: "custom",
+				proxyId: "64b7a1f2c9e77a001a2b3c4d",
+			} as any);
+
+			const createdBody = (monitorsRepository.create as jest.Mock).mock.calls[0][0] as Record<string, unknown>;
+			expect(createdBody.proxyId).toBe("64b7a1f2c9e77a001a2b3c4d");
+		});
+
 		it("throws when repository returns null", async () => {
 			const monitorsRepository = createMonitorsRepositoryMock();
 			(monitorsRepository.create as jest.Mock).mockResolvedValue(null);
@@ -750,9 +782,42 @@ describe("MonitorService", () => {
 			const result = await service.editMonitor({ teamId: TEAM_ID, monitorId: MONITOR_ID, body: { name: "Updated" } });
 
 			expect(result).toMatchObject({ name: "Updated" });
-			expect(monitorsRepository.updateById).toHaveBeenCalledWith(MONITOR_ID, TEAM_ID, { name: "Updated" });
+			expect(monitorsRepository.updateById).toHaveBeenCalledWith(MONITOR_ID, TEAM_ID, { name: "Updated" }, { unsetProxyId: false });
 			expect(jobQueue.updateJob).toHaveBeenCalledWith(updatedMonitor);
 		});
+
+		it("keeps proxyId and does not unset it when proxyMode is custom", async () => {
+			const monitorsRepository = createMonitorsRepositoryMock();
+			(monitorsRepository.updateById as jest.Mock).mockResolvedValue(makeMonitor());
+			const { service } = createService({ monitorsRepository });
+
+			await service.editMonitor({
+				teamId: TEAM_ID,
+				monitorId: MONITOR_ID,
+				body: { proxyMode: "custom", proxyId: "64b7a1f2c9e77a001a2b3c4d" } as any,
+			});
+
+			expect(monitorsRepository.updateById).toHaveBeenCalledWith(
+				MONITOR_ID,
+				TEAM_ID,
+				{ proxyMode: "custom", proxyId: "64b7a1f2c9e77a001a2b3c4d" },
+				{ unsetProxyId: false }
+			);
+		});
+
+		it("strips proxyId and unsets it when proxyMode moves off custom", async () => {
+			const monitorsRepository = createMonitorsRepositoryMock();
+			(monitorsRepository.updateById as jest.Mock).mockResolvedValue(makeMonitor());
+			const { service } = createService({ monitorsRepository });
+
+			await service.editMonitor({
+				teamId: TEAM_ID,
+				monitorId: MONITOR_ID,
+				body: { proxyMode: "inherit", proxyId: "64b7a1f2c9e77a001a2b3c4d" } as any,
+			});
+
+			expect(monitorsRepository.updateById).toHaveBeenCalledWith(MONITOR_ID, TEAM_ID, { proxyMode: "inherit" }, { unsetProxyId: true });
+		});
 	});
 
 	describe("updateNotifications", () => {

server/test/unit/validation/monitorValidation.test.ts

@@ -477,6 +477,7 @@ describe("monitorValidation — proxy fields", () => {
 		type: "http" as const,
 		url: "https://example.com",
 	};
+	const PROXY_ID = "64b7a1f2c9e77a001a2b3c4d";
 
 	describe("createMonitorBodyValidation", () => {
 		it("defaults proxyMode to inherit", () => {
@@ -487,23 +488,38 @@ describe("monitorValidation — proxy fields", () => {
 		});
 
 		it("accepts custom mode with a proxyId", () => {
-			const parsed = createMonitorBodyValidation.parse({ ...baseHttpBody, proxyMode: "custom", proxyId: "proxy-1" });
+			const parsed = createMonitorBodyValidation.parse({ ...baseHttpBody, proxyMode: "custom", proxyId: PROXY_ID });
 
 			expect(parsed.proxyMode).toBe("custom");
-			expect(parsed.proxyId).toBe("proxy-1");
+			expect(parsed.proxyId).toBe(PROXY_ID);
 		});
 
 		it("rejects custom mode without a proxyId", () => {
 			expect(() => createMonitorBodyValidation.parse({ ...baseHttpBody, proxyMode: "custom" })).toThrow();
 		});
 
+		it("rejects custom mode with an empty-string proxyId", () => {
+			expect(() => createMonitorBodyValidation.parse({ ...baseHttpBody, proxyMode: "custom", proxyId: "" })).toThrow();
+		});
+
+		it("normalizes an empty-string proxyId to undefined on non-custom modes", () => {
+			for (const proxyMode of ["inherit", "none"] as const) {
+				const parsed = createMonitorBodyValidation.parse({ ...baseHttpBody, proxyMode, proxyId: "" });
+				expect(parsed.proxyId).toBeUndefined();
+			}
+		});
+
+		it("rejects a proxyId that is not a valid ObjectId", () => {
+			expect(() => createMonitorBodyValidation.parse({ ...baseHttpBody, proxyMode: "custom", proxyId: "proxy-1" })).toThrow();
+		});
+
 		it("rejects an unknown proxyMode", () => {
 			expect(() => createMonitorBodyValidation.parse({ ...baseHttpBody, proxyMode: "global" })).toThrow();
 		});
 
 		it("allows a stray proxyId on non-custom modes", () => {
 			for (const proxyMode of ["inherit", "none"] as const) {
-				const parsed = createMonitorBodyValidation.parse({ ...baseHttpBody, proxyMode, proxyId: "proxy-1" });
+				const parsed = createMonitorBodyValidation.parse({ ...baseHttpBody, proxyMode, proxyId: PROXY_ID });
 				expect(parsed.proxyMode).toBe(proxyMode);
 			}
 		});
@@ -521,9 +537,19 @@ describe("monitorValidation — proxy fields", () => {
 		});
 
 		it("accepts custom mode with a proxyId", () => {
-			const parsed = editMonitorBodyValidation.parse({ proxyMode: "custom", proxyId: "proxy-1" });
+			const parsed = editMonitorBodyValidation.parse({ proxyMode: "custom", proxyId: PROXY_ID });
 
-			expect(parsed.proxyId).toBe("proxy-1");
+			expect(parsed.proxyId).toBe(PROXY_ID);
+		});
+
+		it("normalizes an empty-string proxyId to undefined", () => {
+			const parsed = editMonitorBodyValidation.parse({ proxyMode: "inherit", proxyId: "" });
+
+			expect(parsed.proxyId).toBeUndefined();
+		});
+
+		it("rejects a proxyId that is not a valid ObjectId", () => {
+			expect(() => editMonitorBodyValidation.parse({ proxyMode: "custom", proxyId: "not-an-object-id" })).toThrow();
 		});
 	});