fix: resolve CodeQL alerts #24 and #26#4145
Merged
khassel merged 2 commits intoMagicMirrorOrg:developfrom May 6, 2026
Merged
Conversation
…eQL #26) startApplication hardcoded port 8080, so the "MM_PORT override" test was actually testing port 8080 all along and passing for the wrong reason. Also removes the dead exec callback that masked the issue.
khassel
approved these changes
May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#24 –
js/class.jsfnTestworks by serialising a function to a string and checking if"xyz"appears in it - the function is never actually called. The barexyz;is never executed, so CodeQL is right to flag it.return xyz;makes the intent clear. So this is purely a cosmetic change.#26 –
tests/e2e/helpers/global-setup.jsCodeQL flagged
if (exec) exec;as a useless expression - and it was right. But the real find was one level deeper.startApplicationhardcodedconst port = 8080, soMM_PORTwas 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
execparameter, madestartApplicationreadMM_PORTfrom the environment, and fixed the test so it actually checks what it says it checks.