diff --git a/client/src/Validation/monitor.ts b/client/src/Validation/monitor.ts index 5b53ff3a5..00ab3aaff 100644 --- a/client/src/Validation/monitor.ts +++ b/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 }), diff --git a/server/src/api/validation/monitorValidation.ts b/server/src/api/validation/monitorValidation.ts index 65a64ed10..e694a216d 100644 --- a/server/src/api/validation/monitorValidation.ts +++ b/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(), diff --git a/server/src/domain/monitors/monitor.repository.interface.ts b/server/src/domain/monitors/monitor.repository.interface.ts index 1ce2b8fa8..1b12fef71 100644 --- a/server/src/domain/monitors/monitor.repository.interface.ts +++ b/server/src/domain/monitors/monitor.repository.interface.ts @@ -34,7 +34,7 @@ export interface IMonitorsRepository { findByIds(monitorIds: string[], options?: { recentChecks?: RecentChecksMode }): Promise; // update - updateById(monitorId: string, teamId: string, updates: Partial): Promise; + updateById(monitorId: string, teamId: string, updates: Partial, options?: { unsetProxyId?: boolean }): Promise; updateByIds(monitorIds: string[], teamId: string, updates: Partial, excludeStatuses?: MonitorStatus[]): Promise; updateStatusWindowAndChecks( monitorId: string, diff --git a/server/src/domain/monitors/monitor.repository.mongo.ts b/server/src/domain/monitors/monitor.repository.mongo.ts index 6cf2e0ecf..8f773f983 100644 --- a/server/src/domain/monitors/monitor.repository.mongo.ts +++ b/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) => { - const updatedMonitor = await MonitorModel.findOneAndUpdate( - { _id: monitorId, teamId }, - { - $set: { - ...patch, - }, - }, - { new: true, runValidators: true } - ); + updateById = async (monitorId: string, teamId: string, patch: Partial, options?: { unsetProxyId?: boolean }) => { + const fields: Partial = { ...patch }; + const update: Record = { $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 }); } diff --git a/server/src/domain/monitors/monitor.service.ts b/server/src/domain/monitors/monitor.service.ts index 31488c378..6cf9af54b 100644 --- a/server/src/domain/monitors/monitor.service.ts +++ b/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 => { + // 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 }) => { - 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; }; diff --git a/server/test/unit/services/monitorService.test.ts b/server/test/unit/services/monitorService.test.ts index 0f56e5d2b..fcfdf533d 100644 --- a/server/test/unit/services/monitorService.test.ts +++ b/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; + 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; + 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", () => { diff --git a/server/test/unit/validation/monitorValidation.test.ts b/server/test/unit/validation/monitorValidation.test.ts index 4de4538e1..ea2257072 100644 --- a/server/test/unit/validation/monitorValidation.test.ts +++ b/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(); }); });