From 9386c44928d5b8a930daa8cfd0caeee35500a4c7 Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Thu, 7 May 2026 00:28:02 +0200 Subject: [PATCH] fix: resolve CodeQL alerts #24 and #26 (#4145) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **[#24](https://github.com/MagicMirrorOrg/MagicMirror/security/code-scanning/24) – `js/class.js`** `fnTest` works by serialising a function to a string and checking if `"xyz"` appears in it - the function is never actually called. The bare `xyz;` is never executed, so CodeQL is right to flag it. `return xyz;` makes the intent clear. So this is purely a cosmetic change. **[#26](https://github.com/MagicMirrorOrg/MagicMirror/security/code-scanning/26) – `tests/e2e/helpers/global-setup.js`** CodeQL flagged `if (exec) exec;` as a useless expression - and it was right. But the real find was one level deeper. `startApplication` hardcoded `const port = 8080`, so `MM_PORT` was always overwritten before the app started. The test named "Set port 8100 on environment variable MM_PORT" was actually testing port 8080 the whole time - it just happened to pass anyway. Removed the dead `exec` parameter, made `startApplication` read `MM_PORT` from the environment, and fixed the test so it actually checks what it says it checks. --- js/class.js | 2 +- tests/e2e/helpers/global-setup.js | 10 +++++----- tests/e2e/port_spec.js | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/js/class.js b/js/class.js index 9cfeee4c..f0196d5a 100644 --- a/js/class.js +++ b/js/class.js @@ -11,7 +11,7 @@ (function () { let initializing = false; const fnTest = (/xyz/).test(function () { - xyz; + return xyz; }) ? /\b_super\b/ : /.*/; diff --git a/tests/e2e/helpers/global-setup.js b/tests/e2e/helpers/global-setup.js index 8bbb810b..01e20fff 100644 --- a/tests/e2e/helpers/global-setup.js +++ b/tests/e2e/helpers/global-setup.js @@ -88,7 +88,7 @@ exports.getPage = () => { return page; }; -exports.startApplication = async (configFilename, exec) => { +exports.startApplication = async (configFilename) => { vi.resetModules(); // Clear Node's require cache for config and app files to prevent stale configs and middlewares @@ -107,8 +107,8 @@ exports.startApplication = async (configFilename, exec) => { await exports.stopApplication(); } - // Use fixed port 8080 (tests run sequentially, no conflicts) - const port = 8080; + // Use MM_PORT if preset by a test, otherwise default to 8080. + const port = Number(process.env.MM_PORT) || 8080; global.testPort = port; // Set config sample for use in test @@ -121,12 +121,11 @@ exports.startApplication = async (configFilename, exec) => { process.env.MM_CONFIG_FILE = configPath; - // Override port in config - MUST be set before app loads + // Ensure MM_PORT is set before app loads process.env.MM_PORT = port.toString(); process.env.mmTestMode = "true"; process.setMaxListeners(0); - if (exec) exec; global.app = require(`${global.root_path}/js/app`); return global.app.start(); @@ -143,6 +142,7 @@ exports.stopApplication = async (waitTime = 100) => { await global.app.stop(); delete global.app; delete global.testPort; + delete process.env.MM_PORT; // Wait for any pending async operations to complete before closing DOM await new Promise((resolve) => setTimeout(resolve, waitTime)); diff --git a/tests/e2e/port_spec.js b/tests/e2e/port_spec.js index a64ac568..341becc7 100644 --- a/tests/e2e/port_spec.js +++ b/tests/e2e/port_spec.js @@ -11,15 +11,15 @@ describe("port directive configuration", () => { }); it("should return 200", async () => { - const port = global.testPort || 8080; - const res = await fetch(`http://localhost:${port}`); + const res = await fetch(`http://localhost:${global.testPort}`); expect(res.status).toBe(200); }); }); describe("Set port 8100 on environment variable MM_PORT", () => { beforeAll(async () => { - await helpers.startApplication("tests/configs/port_8090.js", (process.env.MM_PORT = 8100)); + process.env.MM_PORT = "8100"; + await helpers.startApplication("tests/configs/port_8090.js"); }); afterAll(async () => { @@ -27,8 +27,8 @@ describe("port directive configuration", () => { }); it("should return 200", async () => { - const port = global.testPort || 8080; - const res = await fetch(`http://localhost:${port}`); + expect(global.testPort).toBe(8100); + const res = await fetch(`http://localhost:${global.testPort}`); expect(res.status).toBe(200); }); });