Security issues with the install script

[rendler-denis]

Security issues with the install script:

1. Telemetry on by default:

Why is there a need to send this payload? And why is there a need to hide its output? IT should default to --no-telemetry not the other way around

2. Grants passwordless root to the app user

echo "$user ALL=(ALL) NOPASSWD:ALL" >/etc/sudoers.d/$user

This is basically full root with no prompts.

Do you really need a new user with sudo since you already add it to the docker group?

3. don't install automatically Docker plugins. they have the unintended consequence of blocking everything in case of issues. additionally, I might be monitoring my Docker setup with other tools.

[hunvreus]

I wasn’t hiding it, I just pushed it quickly as I was trying to figure out a way to track versions to see what I had to support in my updates. I released this thing in beta a few weeks ago and didn’t expect to get that much attention.

In the upcoming version (see development branch) I rewrote all of the scripts. The installer now comes with a flag to disable telemetry:

https://github.com/hunvreus/devpush/blob/development/scripts/prod/install.sh#L39

[rendler-denis]

hi @hunvreus .

your point is fair enough. what threw me off was the output of curl being sent to /de/null. given the stuff companies nowadays are trying to pull, I am a bit suspicious about telemetry being sent without any message.
I do understand that initially my point came out differently than I wanted, that's why I rewrote it, but the points mentioned are still valid in my opinion.
I did take a look at the scripts in development, and still see the same issues:

• telemetry is still being sent by default, especially when following the instructions in the readme. I do understand the need, but I believe properly informing users beforehand and giving them the opportunity to opt-in creates a lot more confidence and trust. in any case, if someone has a problem they will list their version anyways.
• you are still adding the NOPASSWD to the new user which is a high vulnerability (https://github.com/hunvreus/devpush/blob/bcff501f3bdc44cddbbc7e090ccc2a787960dcfb/scripts/prod/install.sh#L212C27-L212C38) . I understand the user will need access to Docker, but once added to the Docker group there is no need for it to have sudo privs, especially without password.
• and still installing Loki with no way of opting out. and from the scripts I suspect you already found out that if a plugin is crashing, the entire Docker platform will stop working. Imagine deploying this along side a few dozen production apps and suddenly after an upgrade, the Docker engine is not compatible with the plugin and nothing comes up. It's going to take a while to debug this scenario. ask me how I know that 🥲 .
• additionally, unattended-upgrades is a nightmare waiting to happen if managing a fleet of servers with diverse apps and configs. while on paper sounds great I often found myself with alerts that servers have not rebooted after an update.

at the end of the day, these points are just recommendations and is what stops me from deploying this app, although I find it very interesting and might be useful in a few cases I'm thinking of.
But, if you don't find any value, just close the ticket and I'll just move on. 😃

[hunvreus]

All good, it's helpful:

• Telemetry on by default: The new install script does still default to telemetry on, but it does also tells the user it's sending data. I plan on having a callout on the install page giving the user the optional one liner with --no-telemetry. Telemetry is helpful (and anonymized) for me, but I get it.
• Passwordless sudo for devpush user: I'll see if I can remove passwordless sudo, I just need to clean some of the file permissions (e.g. config.json, version.json, certificates...).
• Docker plugin: It's only there because I had some issues with Promtail initially, and I wanted to ship something. The Loki driver is an issue anyway as it prevents me from offering ARM support, so it needs to go. I'll see if I can make it in the next release.

[hunvreus]

Hey there, I've made a fair amount of changes in the latest release: https://github.com/hunvreus/devpush/releases/tag/0.2.1

In particular:

• Telemetry can be switched off and is clearly outlined in the install output.
• devpush doesn't have passwordless sudo anymore.
• The Loki driver was removed (I'm now using Alloy)

The docs also explain how to install things yourself if you decide to do so: https://devpu.sh/docs/installation/#manual-installation

Let me know if you think this addresses your main concerns.