[Request] Don't install modules at runtime

[kaysond]

Background

I just got some micro desktops with vPro support, and I wanted to install MeshCentral using the Docker image in uldiseihenbergs/meshcentral-builds. A best practice for Docker containers is to run services as a unique user (particularly, non-root), which can be accomplished with --user or user: in a docker compose file. Unfortunately, doing that didn't work because of a permissions issue with "module installation".

I dug into the code and discovered that dependencies are being gathered and npm install is being executed by the main script at runtime:

MeshCentral/meshcentral.js

Lines 4019 to 4107 in 990da39

	// Build the list of required modules
	// NOTE: ALL MODULES MUST HAVE A VERSION NUMBER AND THE VERSION MUST MATCH THAT USED IN Dockerfile
	var modules = ['archiver@5.3.2','body-parser@1.20.2','cbor@5.2.0','compression@1.7.4','cookie-session@2.0.0','express@4.18.2','express-handlebars@5.3.5','express-ws@4.0.0','ipcheck@0.1.0','minimist@1.2.8','multiparty@4.2.3','@yetzt/nedb','node-forge@1.3.1','ua-parser-js@1.0.37','ws@8.14.2','yauzl@2.10.0'];
	if (require('os').platform() == 'win32') { modules.push('node-windows@0.1.14'); modules.push('loadavg-windows@1.1.1'); if (sspi == true) { modules.push('node-sspi@0.2.10'); } } // Add Windows modules
	if (ldap == true) { modules.push('ldapauth-fork@5.0.5'); }
	if (ssh == true) { modules.push('ssh2@1.15.0'); }
	if (passport != null) { modules.push(...passport); }
	if (captcha == true) { modules.push('svg-captcha@1.4.0'); }
	if (sessionRecording == true) { modules.push('image-size@1.0.2'); } // Need to get the remote desktop JPEG sizes to index the recodring file.
	if (config.letsencrypt != null) { modules.push('acme-client@4.2.5'); } // Add acme-client module. We need to force v4.2.4 or higher since olver versions using SHA-1 which is no longer supported by Let's Encrypt.
	if (config.settings.mqtt != null) { modules.push('aedes@0.39.0'); } // Add MQTT Modules
	if (config.settings.mysql != null) { modules.push('mysql2@3.6.2'); } // Add MySQL.
	//if (config.settings.mysql != null) { modules.push('@mysql/xdevapi@8.0.33'); } // Add MySQL, official driver (https://dev.mysql.com/doc/dev/connector-nodejs/8.0/)
	if (config.settings.mongodb != null) { modules.push('mongodb@4.13.0'); modules.push('saslprep@1.0.3'); } // Add MongoDB, official driver.
	if (config.settings.postgres != null) { modules.push('pg@8.7.1'); modules.push('pgtools@0.3.2'); } // Add Postgres, Postgres driver.
	if (config.settings.mariadb != null) { modules.push('mariadb@3.2.2'); } // Add MariaDB, official driver.
	if (config.settings.acebase != null) { modules.push('acebase@1.29.5'); } // Add AceBase, official driver.
	if (config.settings.sqlite3 != null) { modules.push('sqlite3@5.1.6'); } // Add sqlite3, official driver.
	if (config.settings.vault != null) { modules.push('node-vault@0.10.2'); } // Add official HashiCorp's Vault module.
	if (config.settings.plugins != null) { modules.push('semver@7.5.4'); } // Required for version compat testing and update checks
	if ((config.settings.plugins != null) && (config.settings.plugins.proxy != null)) { modules.push('https-proxy-agent@7.0.2'); } // Required for HTTP/HTTPS proxy support
	else if (config.settings.xmongodb != null) { modules.push('mongojs@3.1.0'); } // Add MongoJS, old driver.
	if (nodemailer || ((config.smtp != null) && (config.smtp.name != 'console')) || (config.sendmail != null)) { modules.push('nodemailer@6.9.8'); } // Add SMTP support
	if (sendgrid || (config.sendgrid != null)) { modules.push('@sendgrid/mail'); } // Add SendGrid support
	if ((args.translate || args.dev) && (Number(process.version.match(/^v(\d+\.\d+)/)[1]) >= 16)) { modules.push('jsdom@22.1.0'); modules.push('esprima@4.0.1'); modules.push('minify-js@0.0.4'); modules.push('html-minifier@4.0.0'); } // Translation support
	if (typeof config.settings.crowdsec == 'object') { modules.push('@crowdsec/express-bouncer@0.1.0'); } // Add CrowdSec bounser module (https://www.npmjs.com/package/@crowdsec/express-bouncer)
	if (typeof config.settings.autobackup == 'object') {
	// Setup encrypted zip support if needed
	if (config.settings.autobackup.zippassword) { modules.push('archiver-zip-encrypted@1.0.11'); }
	// Enable Google Drive Support
	if (typeof config.settings.autobackup.googledrive == 'object') { modules.push('googleapis@128.0.0'); }
	// Enable WebDAV Support
	if (typeof config.settings.autobackup.webdav == 'object') {
	if ((typeof config.settings.autobackup.webdav.url != 'string') || (typeof config.settings.autobackup.webdav.username != 'string') || (typeof config.settings.autobackup.webdav.password != 'string')) { addServerWarning("Missing WebDAV parameters.", 2, null, !args.launch); } else { modules.push('webdav@4.11.3'); }
	}
	}
	// Setup common password blocking
	if (wildleek == true) { modules.push('wildleek@2.0.0'); }
	// Setup 2nd factor authentication
	if (config.settings.no2factorauth !== true) {
	// Setup YubiKey OTP if configured
	if (yubikey == true) { modules.push('yubikeyotp@0.2.0'); } // Add YubiKey OTP support
	if (allsspi == false) { modules.push('otplib@10.2.3'); } // Google Authenticator support (v10 supports older NodeJS versions).
	}
	// Desktop multiplexor support
	if (config.settings.desktopmultiplex === true) { modules.push('image-size@1.0.2'); }
	// SMS support
	if (config.sms != null) {
	if (config.sms.provider == 'twilio') { modules.push('twilio@4.19.0'); }
	if (config.sms.provider == 'plivo') { modules.push('plivo@4.58.0'); }
	if (config.sms.provider == 'telnyx') { modules.push('telnyx@1.25.5'); }
	}
	// Messaging support
	if (config.messaging != null) {
	if (config.messaging.telegram != null) { modules.push('telegram@2.19.8'); modules.push('input@1.0.1'); }
	if (config.messaging.discord != null) { if (nodeVersion >= 17) { modules.push('discord.js@14.6.0'); } else { delete config.messaging.discord; addServerWarning('This NodeJS version does not support Discord.js.', 26); } }
	if (config.messaging.xmpp != null) { modules.push('@xmpp/client@0.13.1'); }
	if (config.messaging.pushover != null) { modules.push('node-pushover@1.0.0'); }
	if (config.messaging.zulip != null) { modules.push('zulip@0.1.0'); }
	}
	// Setup web based push notifications
	if ((typeof config.settings.webpush == 'object') && (typeof config.settings.webpush.email == 'string')) { modules.push('web-push@3.6.6'); }
	// Firebase Support
	// Avoid 0.1.8 due to bugs: https://github.com/guness/node-xcs/issues/43
	if (config.firebase != null) { modules.push('node-xcs@0.1.7'); }
	// Syslog support
	if ((require('os').platform() != 'win32') && (config.settings.syslog || config.settings.syslogjson)) { modules.push('modern-syslog@1.2.0'); }
	if (config.settings.syslogtcp) { modules.push('syslog@0.1.1-1'); }
	// Setup heapdump support if needed, useful for memory leak debugging
	// https://www.arbazsiddiqui.me/a-practical-guide-to-memory-leaks-in-nodejs/
	if (config.settings.heapdump === true) { modules.push('heapdump@0.3.15'); }
	// Install any missing modules and launch the server
	InstallModules(modules, args, function () {
	if (require('os').platform() == 'win32') { try { require('node-windows'); } catch (ex) { console.log("Module node-windows can't be loaded. Restart MeshCentral."); process.exit(); return; } }
	meshserver = CreateMeshCentralServer(config, args);
	meshserver.Start();
	});

Docker Issues

Permissions

The above means that the permissions issue in Docker can never really be resolved. The image must be created with a particular user selected at build time, and that generally should be root. The container should then ideally be able to run as any user, but in order to be able do an npm install at run time, the build user would have to be chosen in advance to be the same as the run user. (Or you could grant everyone write permissions, but that's not a great choice for security).

Non-ephemeral container

There's also a separate but related issue when it comes to Docker, and that is that you should not write directly to the container's writable layer because 1) the container is ephemeral and should contain everything needed to run the application, and 2) for performance reasons. You can get around this with a volume, but then you run into issues with volume persistence in addition to violating Docker image design principles.

Dangers With Runtime Module Installation

I did a lot of searching, and I couldn't find any other npm packages, services, etc, that install modules at runtime in this way. From my own experience, and according to the nodejs docs, this is a bad practice. It's fragile in that you have to maintain the main dependency list in two places (package.json and meshcentral.js), and from looking through the issues it seems that module installation issues come up frequently. The whole point of npm is to resolve dependencies and install packages from a package.json (really: package-lock.json). It's made to take care of this for you, and if you properly define all of your dependencies I would guess you'll get far fewer reports of issues with installation and update because all of the versioning is frozen and reproducible.

Disk Space

I'm curious why it was done this way. I'm guessing it's to save the disk space of modules that aren't actually used by the config, and I am seeing 21MB on a "default" install, and 357MB with all modules installed, and 100MB of that is googleapis. Nonetheless, there's a danger in modifying your "executable" based on user configuration, and I don't think even the full 357MB is so concerning that it's worth trying to save.

Request

I think the project (and Docker users!) would benefit from moving all of the dependencies into package.json and removing the runtime install code. I'd also be happy to contribute the change, including documentation updates, etc. Another benefit of doing this is it makes installing and running meshcentral trivial with npx

Bonus: I'd be happy to contribute an official docker container too, including github actions to automatically publish it.

The exception to the rule

npm doesn't support os-specific dependencies, so you can't add windows-only packages to dependencies or Unix systems will fail installation. You would add it to optionalDependencies, where the failure won't prevent Unix/Mac installs, but windows users should get the packages. There is some risk that those packages fail to install on windows for some other reason, so a runtime check might be warranted.

[kaysond]

PS: Thanks for reading my essay :)

[si458]

https://github.com/uldiseihenbergs/meshcentral-builds/ is an unofficial docker image...
Nothing to do with meshcentral

Our official one is listed on the main repo under packages right hand side
https://github.com/Ylianst/MeshCentral/pkgs/container/meshcentral

All the docker code is here
https://github.com/Ylianst/MeshCentral/tree/master/docker
https://github.com/Ylianst/MeshCentral/blob/master/.github/workflows/docker.yml

[si458]

If you think u can improve our docker image then please have at it!
More than happy to accept PR requests...

Edit. I did originally try writing a small docker image but then realised people needed extra stuff like for example someone ran the docker image with syslogtcp which required python in the docker image, so we had to include that which bumped up the size.
I would like to try creating separate images at some point mc/mongodb mc/mysql mc/python etc if that makes sense?

[Jamesits]

This is still an issue for both container image users. To elaborate:

• The container have to be run as root (non-root containers are enforced by a lot companys) (e.g. Docker on Debian 11 fails on version 1.1.15 and 1.1.16 giving NPM errors #5545 )
• Supply chain attack surface (no way to hash-check the NPM packages)
• The container image does not work offline at all (e.g. OIDC cannot be used in an offline (docker) environment #6568 )
• Additional network traffic on each container restart
• Slow start time

Plus additional trouble for the project maintainers:

• You have to remember to update package versions in meshcentral.js and Dockerfile and package.json, and they have to match, otherwise you get undefined/untested behaviors (e.g. MeshCentral fails to start with openid-client v6.0.0 #6459 Meshcentral in Docker on Ubuntu 20.04 not able to update past 1.1.14 #5640 Update process finish with no errors but that's not correct #4471 DesktopMulitplex Error: Cannot find module 'image-size' #2512 Unable to upgrade version #1957 1.1.16 -> 1.1.18 missing \node_modules\node-windows\ #5694 ...)

The solution: just add all the packages into package.json and generate a lock file with the package manager of your choice! Then remove the package checks completely from meshcentral.js.

you can't add windows-only packages to dependencies or Unix systems will fail installation.

All the Windows-only packages correctly installs under Linux and macOS! I've tested that. If you want to try for yourself, you can clone https://github.com/Jamesits/docker-meshcentral2.git on your OS and do a pnpm install (outside Docker).

for example someone ran the docker image with syslogtcp which required python in the docker image…
I would like to try creating separate images at some point mc/mongodb mc/mysql mc/python etc if that makes sense

No, this is not the way to do it. Your container image should only contain everything your program need to run. For example, if you need to call mongodb client tools or other database client for daily backup (a function of your program), then include them. syslogtcp or even python is not of one of these cases.

If an user want to collect syslog of a container, they are better off doing it in a way that is non-intrusive. There are a lot of ways to achieve this. One common way is to configure an alternative logging driver on their container runtime; another common choice is to use a sidecar container.

Containers are designed to be inherited. If an user really need a certain tool they use inside the container, they can simply add layers to your container with the following Dockerfile:

FROM your-image:tag
RUN ...

This is cleaner and more managable than tell the upstream packager to include the universe in their container image. Plus, large container images is not a problem. OCI container images are designed to share layers with each other to reduce disk usage, and ~1GiB is not an extreme size for such a container.

---

Besides that, MeshCentral tries to find its config files relative to its code which does not work if the node_modules is not a flatten one. Not all npm package manager create a flattened node_modules, and an NPM package should not change anything inside the node_modules. Doing that is very dangerous and potentially disruptive. (e.g. #4318 )

---

The way that the official container image being constructed has brought much trouble to me, so much that I had to maintain my own container image for MeshCentral. Here's my approach, for reference:

• Toolchain: corepack + pnpm (just what I'm most familiar with)
• All packages locked and hash-checked during a CI container build
• A script to extract the dependency packages automatically (albeit not the best way to do it...)
• Upstream updates trigger auto PRs by Renovate bot

More than happy to accept PR requests...

I'd like to contribute, but the changes required exceed what I define as a small progressive patch (small enough so that it has a high chance of being reviewed even if submitted by a random person). And it's the engineering choices I mentioned above that is proactively preventing people from contributing by making any change unneedingly expensive. The same engineering choices lead to a lot trouble and confusion to other users over the years, no matter if they use MeshCentral inside a container or not. And it cost you guys time to deal with these user confusion and issues.

Maybe it's time to fix that.

(Don't know how to write an ending for this comment. Sorry.)

[DaanSelen]

Is this still an issue?

[Jamesits]

@DaanSelen Yes, it is still an issue.

[DaanSelen]

And assuming this is also the case for the PR in progress? #6937

[Jamesits]

@DaanSelen Thank you! It seems that PR does not solve the original problem described by this issue (dynamic node package installation at runtime) though?

[DaanSelen]

Possibly! But then i will try to fix it there 😁

[si458]

im not sure how people would like us to resolve this issue?
we cant include every single npm package in the package.json
because the size/package count of meshcentral would get silly and out of control!
as is at the moment the new docker image size is 1GB and that includes the 350MB JUST FOR MESHCENTRAL CODE!!!
@DaanSelen did an amazing job of changing the docker image so it now includes database npms by default for everyone
but again if u want certin feature it still needs to install the missing packages
at least if you do a normal npm install its only once it needs to do this!
but with docker because the image is expendable, it has to run it every time im afraid

[Jamesits]

What's wrong with 350MB of nodejs code? I mean, if you chose the nodejs ecosystem, you should fully accept its drawbacks, like the dependency hell and a large runtime. These packages are going to be installed on the user's computers anyway. (The packages are around 250MB actually, if it makes you feel better.)

And most Docker users would like a slight larger size image more than having to manage language-specific dependencies inside the container. Docker is invented exactly for this case. No manual dependency management, just immutable images shipped and ready to run.

at least if you do a normal npm install its only once it needs to do this!

No, as Docker images are somewhat immutable, these packages are going to be re-downloaded every time the container is restarted. This is a magnitude worse than including them in the image. (Trying to persist these files comes with another set of different challenges.)

If multiple different database variants are viable, try Docker multistage build with different dependencies packaged to different targets, and distributed with different tag suffixes. This is the equivalent of "downloading the packages you need only". It reduces the image size one user needed to download, but is a little more complex to implement.

[si458]

@Jamesits i decided that hopefully before the next release this week,
im going to add in another docker image, something like latest-full
and this will include EVERY SINGLE NPM PACKAGE we use on the project
then if people want to download the full package they can do 👍
hope this is ok?

its also already on my to-do to add in the multi database tags too
so it would be something like latest-mysql, latest-postgres, latest-mongodb
and each will include the npm package that needs for that database!

[Jamesits]

Thank you, this would be extra helpful!