Cleanup comments and switch to integration tests
This commit is contained in:
parent
6aa7a74664
commit
15de932623
@ -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<void>} Resolves on success, rejects on error.
|
||||
* @returns {Promise<void>}
|
||||
*/
|
||||
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";
|
||||
|
||||
@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user