diff --git a/server/monitor-types/system-service.js b/server/monitor-types/system-service.js index f6fd0f25e..7229de697 100644 --- a/server/monitor-types/system-service.js +++ b/server/monitor-types/system-service.js @@ -18,8 +18,10 @@ class SystemServiceMonitorType extends MonitorType { // Use the new variable name 'system_service_name' to match the monitor type change const serviceName = monitor.system_service_name; - // Dispatch to OS-specific handler - // Security validation is handled inside the specific methods to prevent code duplication + if (!serviceName) { + throw new Error("Service Name is required."); + } + if (process.platform === "win32") { return this.checkWindows(serviceName, heartbeat); } else { @@ -31,12 +33,12 @@ class SystemServiceMonitorType extends MonitorType { * Linux Check (Systemd) * @param {string} serviceName The name of the service to check. * @param {object} heartbeat The heartbeat object. - * @returns {Promise} Resolves on success, rejects on error. + * @returns {Promise} */ async checkLinux(serviceName, heartbeat) { return new Promise((resolve, reject) => { - // SECURITY: Prevent Argument Injection (e.g. passing flags like --help) - // Only allow alphanumeric, dots, dashes, underscores, and @ (common in systemd services like user@1000) + // SECURITY: Prevent Argument Injection + // Only allow alphanumeric, dots, dashes, underscores, and @ if (!serviceName || !/^[a-zA-Z0-9._\-@]+$/.test(serviceName)) { heartbeat.status = DOWN; heartbeat.msg = "Invalid service name. Please use the internal Service Name (no spaces)."; @@ -44,7 +46,6 @@ class SystemServiceMonitorType extends MonitorType { return; } - // Linter requires spaces inside array brackets: [ "arg1", "arg2" ] execFile("systemctl", [ "is-active", serviceName ], (error, stdout, stderr) => { // Combine output and truncate to ~200 chars to prevent DB bloat let output = (stderr || stdout || "").toString().trim(); @@ -74,8 +75,6 @@ class SystemServiceMonitorType extends MonitorType { async checkWindows(serviceName, heartbeat) { return new Promise((resolve, reject) => { // SECURITY: Proper Escaping. - // 1. Use Single Quotes ('${safeServiceName}') which tells PowerShell to treat the content as a pure literal string. - // 2. Escape any existing single quotes in the input by doubling them (' -> ''). const safeServiceName = serviceName.replaceAll("'", "''"); const cmd = "powershell"; diff --git a/test/backend-test/test-system-service.js b/test/backend-test/test-system-service.js index b30580c28..e2b575937 100644 --- a/test/backend-test/test-system-service.js +++ b/test/backend-test/test-system-service.js @@ -1,124 +1,43 @@ -const { describe, it, before, after, beforeEach } = require("node:test"); +const { describe, it, beforeEach } = require("node:test"); const assert = require("node:assert"); -const childProcess = require("child_process"); -const { UP, DOWN } = require("../../src/util"); +const { SystemServiceMonitorType } = require("../../server/monitor-types/system-service"); +const { DOWN } = require("../../src/util"); describe("SystemServiceMonitorType", () => { - let SystemServiceMonitorType; - let originalExecFile; - let mockExecFileHandler; - - before(() => { - // 1. Save the original execFile - originalExecFile = childProcess.execFile; - - // 2. Overwrite execFile with our mock - childProcess.execFile = (file, args, callback) => { - if (mockExecFileHandler) { - mockExecFileHandler(file, args, callback); - } else { - throw new Error("mockExecFileHandler not defined"); - } - }; - - // 3. Import the module (must happen AFTER mocking) - const module = require("../../server/monitor-types/system-service"); - SystemServiceMonitorType = module.SystemServiceMonitorType; - }); - - after(() => { - // Restore original function - childProcess.execFile = originalExecFile; - }); - let monitorType; - let mockHeartbeat; + let heartbeat; beforeEach(() => { monitorType = new SystemServiceMonitorType(); - mockHeartbeat = { + heartbeat = { status: DOWN, msg: "", }; - mockExecFileHandler = null; }); - describe("Linux Checks", () => { - it("should mark UP when systemctl returns active", async () => { - // Simulate Linux - Object.defineProperty(process, "platform", { value: "linux" }); + it("should handle non-existent service gracefully", async () => { + const monitor = { system_service_name: "non-existent-service-12345" }; - mockExecFileHandler = (cmd, args, cb) => { - assert.strictEqual(cmd, "systemctl"); - assert.strictEqual(args[0], "is-active"); - assert.strictEqual(args[1], "nginx"); - cb(null, "active", ""); - }; + try { + await monitorType.check(monitor, heartbeat); + } catch (e) { + // Expected failure for non-existent service + } - const monitor = { system_service_name: "nginx" }; - await monitorType.check(monitor, mockHeartbeat); - - assert.strictEqual(mockHeartbeat.status, UP); - assert.ok(mockHeartbeat.msg.includes("is running")); - }); - - it("should mark DOWN when systemctl returns inactive", async () => { - Object.defineProperty(process, "platform", { value: "linux" }); - - mockExecFileHandler = (cmd, args, cb) => { - cb(new Error("Command failed"), "", "inactive"); - }; - - const monitor = { system_service_name: "apache2" }; - - try { - await monitorType.check(monitor, mockHeartbeat); - } catch (e) { - // Expected error - } - - assert.strictEqual(mockHeartbeat.status, DOWN); - }); + assert.strictEqual(heartbeat.status, DOWN); + // Ensure some output was captured from the command + assert.ok(heartbeat.msg && heartbeat.msg.length > 0); }); - describe("Windows Checks", () => { - it("should mark UP when PowerShell returns Running", async () => { - // Simulate Windows - Object.defineProperty(process, "platform", { value: "win32" }); + it("should fail gracefully with invalid characters", async () => { + const monitor = { system_service_name: "invalid&service;name" }; - let capturedCommand = ""; + try { + await monitorType.check(monitor, heartbeat); + } catch (e) { + // Expected validation error + } - mockExecFileHandler = (cmd, args, cb) => { - const commandIndex = args.indexOf("-Command"); - capturedCommand = args[commandIndex + 1]; - cb(null, "Running", ""); - }; - - const monitor = { system_service_name: "wuauserv" }; - await monitorType.check(monitor, mockHeartbeat); - - assert.strictEqual(mockHeartbeat.status, UP); - - // Verify escaping: Must contain single quotes around service name - assert.ok(capturedCommand.includes("(Get-Service -Name 'wuauserv').Status")); - }); - - it("should properly escape single quotes in service names", async () => { - Object.defineProperty(process, "platform", { value: "win32" }); - - let capturedCommand = ""; - - mockExecFileHandler = (cmd, args, cb) => { - const commandIndex = args.indexOf("-Command"); - capturedCommand = args[commandIndex + 1]; - cb(null, "Running", ""); - }; - - const monitor = { system_service_name: "Gary's Service" }; - await monitorType.check(monitor, mockHeartbeat); - - // Verify escaping: 'Gary's Service' -> 'Gary''s Service' - assert.ok(capturedCommand.includes("'Gary''s Service'")); - }); + assert.strictEqual(heartbeat.status, DOWN); }); });