Skip to content

fix(MESHCENT-001): CU-86akbhhdk 2 review findings in winservice.js #164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 1 commit into
base: master
Choose a base branch
from
+64 −56
Draft

fix(MESHCENT-001): CU-86akbhhdk 2 review findings in winservice.js #164

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 64 additions & 56 deletions winservice.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,67 +13,75 @@
/*jshint esversion: 6 */
"use strict";

function start() {

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 winservice.js factory-style function does not follow the CreateX/obj/return-obj pattern

Refactored winservice.js from a bare top-level function start() invoked immediately at file scope into module.exports.CreateWinService = function () { var obj = {}; ... obj.start = function start() {...}; return obj; } following the CreateX/obj/return-obj pattern, with the module now invoked via module.exports.CreateWinService().start(); at the bottom. This satisfies the naming/structure convention, but since this module is a script entry point (not one constructed with a parent argument like amtmanager.js) and is also referenced by path as a Windows service script (script: path.join(__dirname, 'winservice.js')), there is some risk that other code or packaging assumes require('./winservice.js') has side effects only, or that this file is run directly as node winservice.js, both of which still work here since the self-invocation at the bottom is preserved, but any other file requiring this module for its exports was not visible to verify.

🤖 Prompt for AI agents 
In winservice.js around line 16, review and complete this code-review fix: winservice.js factory-style function does not follow the CreateX/obj/return-obj pattern.
What the draft fix changed: Refactored winservice.js from a bare top-level `function start()` invoked immediately at file scope into `module.exports.CreateWinService = function () { var obj = {}; ... obj.start = function start() {...}; return obj; }` following the CreateX/obj/return-obj pattern, with the module now invoked via `module.exports.CreateWinService().start();` at the bottom. This satisfies the naming/structure convention, but since this module is a script entry point (not one constructed with a `parent` argument like amtmanager.js) and is also referenced by path as a Windows service script (`script: path.join(__dirname, 'winservice.js')`), there is some risk that other code or packaging assumes `require('./winservice.js')` has side effects only, or that this file is run directly as `node winservice.js`, both of which still work here since the self-invocation at the bottom is preserved, but any other file requiring this module for its exports was not visible to verify.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 45 low — review closely — react 👍/👎 to teach the reviewer

if (require('os').platform() != 'win32') { console.log('ERROR: Win32 only'); process.exit(255); return; }
module.exports.CreateWinService = function () {
var obj = {};

try {
const fs = require('fs');
const path = require('path');

// Search for meshcentral.js
var cwd = null;
var runarg = null;
if (fs.existsSync(path.join(__dirname, 'meshcentral.js'))) {
runarg = path.join(__dirname, 'meshcentral.js');
cwd = __dirname;
} else if (fs.existsSync(path.join(__dirname, '../node_modules/meshcentral/meshcentral.js'))) {
runarg = path.join(__dirname, '../node_modules/meshcentral/meshcentral.js');
cwd = path.join(__dirname, '..');
} else if (fs.existsSync(path.join(__dirname, '../meshcentral/meshcentral.js'))) {
runarg = path.join(__dirname, '../meshcentral/meshcentral.js');
cwd = path.join(__dirname, '../meshcentral');
} else if (fs.existsSync(path.join(__dirname, '../meshcentral.js'))) {
runarg = path.join(__dirname, '../meshcentral.js');
cwd = path.join(__dirname, '..');
}
if (runarg == null) { console.log('ERROR: Unable to find MeshCentral.js'); process.exit(255); return; }

// Setup libraries
const args = require(path.join(cwd, 'node_modules/minimist'))(process.argv.slice(2));
const nodewindows = require(path.join(cwd, 'node_modules/node-windows'));
const service = nodewindows.Service;
const eventlogger = nodewindows.EventLogger;
const servicelog = new eventlogger('MeshCentral');

// Check if we need to install, start, stop, remove ourself as a background service
if (((args.install == true) || (args.uninstall == true) || (args.start == true) || (args.stop == true) || (args.restart == true))) {
var env = [], xenv = ['user', 'port', 'aliasport', 'mpsport', 'mpsaliasport', 'redirport', 'exactport', 'debug'];
for (var i in xenv) { if (args[xenv[i]] != null) { env.push({ name: 'mesh' + xenv[i], value: args[xenv[i]] }); } } // Set some args as service environement variables.
var svc = new service({ name: 'MeshCentral', description: 'MeshCentral Remote Management Server', script: path.join(__dirname, 'winservice.js'), env: env, wait: 2, grow: 0.5 });
svc.on('install', function () { console.log('MeshCentral service installed.'); svc.start(); });
svc.on('uninstall', function () { console.log('MeshCentral service uninstalled.'); process.exit(); });
svc.on('start', function () { console.log('MeshCentral service started.'); process.exit(); });
svc.on('stop', function () { console.log('MeshCentral service stopped.'); if (args.stop) { process.exit(); } if (args.restart) { console.log('Holding 5 seconds...'); setTimeout(function () { svc.start(); }, 5000); } });
svc.on('alreadyinstalled', function () { console.log('MeshCentral service already installed.'); process.exit(); });
svc.on('invalidinstallation', function () { console.log('Invalid MeshCentral service installation.'); process.exit(); });

if (args.install == true) { try { svc.install(); } catch (e) { logException(e); } }
if (args.stop == true || args.restart == true) { try { svc.stop(); } catch (e) { logException(e); } }
if (args.start == true || args.restart == true) { try { svc.start(); } catch (e) { logException(e); } }
if (args.uninstall == true) { try { svc.uninstall(); } catch (e) { logException(e); } }
return;
}

// This module is only called when MeshCentral is running as a Windows service.
// In this case, we don't want to start a child process, so we launch directly without arguments.
require(runarg).mainStart({ "launch": true });
} catch (ex) { console.log(ex); }
var servicelog = null;

// Logging funtions

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 winservice.js: servicelog referenced in logging helpers before it's defined in outer scope

Fixed the temporal-dead-zone/undefined risk in logInfoEvent/logWarnEvent/logErrorEvent by hoisting var servicelog = null; to the outer CreateWinService function scope (declared before the logging helpers), and changed the inner const servicelog = new eventlogger('MeshCentral'); to a plain assignment servicelog = new eventlogger('MeshCentral'); inside obj.start's try block. Now servicelog is always defined (as null until initialized) before any logging helper can be invoked, so if (servicelog != null) no longer throws a ReferenceError regardless of when/whether the try block reaches that line.

🤖 Prompt for AI agents 
In winservice.js around line 72, review and complete this code-review fix: winservice.js: servicelog referenced in logging helpers before it's defined in outer scope.
What the draft fix changed: Fixed the temporal-dead-zone/undefined risk in `logInfoEvent`/`logWarnEvent`/`logErrorEvent` by hoisting `var servicelog = null;` to the outer `CreateWinService` function scope (declared before the logging helpers), and changed the inner `const servicelog = new eventlogger('MeshCentral');` to a plain assignment `servicelog = new eventlogger('MeshCentral');` inside `obj.start`'s try block. Now `servicelog` is always defined (as `null` until initialized) before any logging helper can be invoked, so `if (servicelog != null)` no longer throws a ReferenceError regardless of when/whether the try block reaches that line.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 65 medium — react 👍/👎 to teach the reviewer

function logException(e) { e += ''; logErrorEvent(e); }
function logInfoEvent(msg) { if (servicelog != null) { servicelog.info(msg); } console.log(msg); }
function logWarnEvent(msg) { if (servicelog != null) { servicelog.warn(msg); } console.log(msg); }
function logErrorEvent(msg) { if (servicelog != null) { servicelog.error(msg); } console.error(msg); }
}

start();
obj.start = function start() {
if (require('os').platform() != 'win32') { console.log('ERROR: Win32 only'); process.exit(255); return; }

try {
const fs = require('fs');
const path = require('path');

// Search for meshcentral.js
var cwd = null;
var runarg = null;
if (fs.existsSync(path.join(__dirname, 'meshcentral.js'))) {
runarg = path.join(__dirname, 'meshcentral.js');
cwd = __dirname;
} else if (fs.existsSync(path.join(__dirname, '../node_modules/meshcentral/meshcentral.js'))) {
runarg = path.join(__dirname, '../node_modules/meshcentral/meshcentral.js');
cwd = path.join(__dirname, '..');
} else if (fs.existsSync(path.join(__dirname, '../meshcentral/meshcentral.js'))) {
runarg = path.join(__dirname, '../meshcentral/meshcentral.js');
cwd = path.join(__dirname, '../meshcentral');
} else if (fs.existsSync(path.join(__dirname, '../meshcentral.js'))) {
runarg = path.join(__dirname, '../meshcentral.js');
cwd = path.join(__dirname, '..');
}
if (runarg == null) { console.log('ERROR: Unable to find MeshCentral.js'); process.exit(255); return; }

// Setup libraries
const args = require(path.join(cwd, 'node_modules/minimist'))(process.argv.slice(2));
const nodewindows = require(path.join(cwd, 'node_modules/node-windows'));
const service = nodewindows.Service;
const eventlogger = nodewindows.EventLogger;
servicelog = new eventlogger('MeshCentral');

// Check if we need to install, start, stop, remove ourself as a background service
if (((args.install == true) || (args.uninstall == true) || (args.start == true) || (args.stop == true) || (args.restart == true))) {
var env = [], xenv = ['user', 'port', 'aliasport', 'mpsport', 'mpsaliasport', 'redirport', 'exactport', 'debug'];
for (var i in xenv) { if (args[xenv[i]] != null) { env.push({ name: 'mesh' + xenv[i], value: args[xenv[i]] }); } } // Set some args as service environement variables.
var svc = new service({ name: 'MeshCentral', description: 'MeshCentral Remote Management Server', script: path.join(__dirname, 'winservice.js'), env: env, wait: 2, grow: 0.5 });
svc.on('install', function () { console.log('MeshCentral service installed.'); svc.start(); });
svc.on('uninstall', function () { console.log('MeshCentral service uninstalled.'); process.exit(); });
svc.on('start', function () { console.log('MeshCentral service started.'); process.exit(); });
svc.on('stop', function () { console.log('MeshCentral service stopped.'); if (args.stop) { process.exit(); } if (args.restart) { console.log('Holding 5 seconds...'); setTimeout(function () { svc.start(); }, 5000); } });
svc.on('alreadyinstalled', function () { console.log('MeshCentral service already installed.'); process.exit(); });
svc.on('invalidinstallation', function () { console.log('Invalid MeshCentral service installation.'); process.exit(); });

if (args.install == true) { try { svc.install(); } catch (e) { logException(e); } }
if (args.stop == true || args.restart == true) { try { svc.stop(); } catch (e) { logException(e); } }
if (args.start == true || args.restart == true) { try { svc.start(); } catch (e) { logException(e); } }
if (args.uninstall == true) { try { svc.uninstall(); } catch (e) { logException(e); } }
return;
}

// This module is only called when MeshCentral is running as a Windows service.
// In this case, we don't want to start a child process, so we launch directly without arguments.
require(runarg).mainStart({ "launch": true });
} catch (ex) { console.log(ex); }
};

return obj;
};

module.exports.CreateWinService().start();