Not as root incompatible with docker-based custompios build

[mitant]

Is your feature request related to a problem? Please describe

When using CustomPiOs to build a custom image with crowsnest as a module, the build fails because of the following lines:

if [[ "${SUDO_USER}" = "root" ]]; then
    not_as_root_msg
    exit 1
fi

Describe the solution you'd like

Allow root to install, generate a warning message instead and let it proceed. Or add a new make target that disables this check, like make install-docker

Describe alternatives you've considered

No response

Additional information

No response

[meteyou]

https://github.com/mainsail-crew/MainsailOS/blob/develop/src/modules/crowsnest/start_chroot_script

This is how we install crowsnest with custompios

[meteyou]

And we also provide a custompios module for it: https://github.com/mainsail-crew/crowsnest/blob/master/custompios/README.md

[mitant]

I use the provided start_chroot_script and config--the problem is that if you build using custompios's instructions for docker-based builds, docker uses the root user to build which conflicts with this check against root

https://github.com/guysoft/CustomPiOS/wiki/Building-with-Docker

[meteyou]

ah ok. then we have to wait for @KwadFan.

[mitant]

I forked the repo and am testing a fix. I think replacing ${SUDO_USER} with ${USER} should be acceptable. ${SUDO_USER} refers to the logged in user and I think will always be root for docker, but if you want to make sure the user executing the command is not root, and is a sudoer, then ${USER} is better, with the SUDO_USER check following left there.

[KwadFan]

I tried to follow your thought process. But it shouldnt matter who builds the image. The custompiosos module skipps checking for root ^^ Otherwise we werent also not able to build Images.

Please attach some kind of log and your config file.

Regards Kwad

[mitant]

from my CustomPiOS build log:

Launch crowsnest install routine ...
sudo: unable to resolve host 8491fdd2a6cb: Name or service not known

    Please do NOT run this script as root!

    Login in as regular user and run with 'sudo make install'

make: *** [Makefile:17: install] Error 1
++++ echo_red 'build failed, unmounting image...'
++++ echo -e -n '\e[91m'
++++ echo build failed, unmounting image...
build failed, unmounting image...
++++ echo -e -n '\e[0m'

---

${SUDO_USER} evaluates to root when building from docker, so in the install.sh it triggers the not_as_root_msg, even if i call make install as non-root, i.e, sudo -u SOMEOTHERSUDOER make install.

can you clarify what you mean that the custompios module skips checking for root? the custompios module calls make install, which calls install.sh that has the check for root?

[mitant]

I will have a PR for you soon to review, hopefully it will clarify things.
It contains two changes so far:

1. install.sh ${SUDO_USER} replaced with ${USER} when checking for root.
2. tools/lib/core.sh apt-get is missing a sudo

[mitant]

btw, if you are not building the pi image from a docker container, you will not have run into this issue. this is a docker-based build specific issue

[mitant]

#142

[mitant]

@KwadFan #142 ready for your review

[mryel00]

I finally found time to test the docker setup and fiddle around with it a little bit.

the changes are to point to my fork and to call sudo -u "${BASE_USER}" make install in the crowsnest start_chroot_script

You mentioned this in #142, so did you change the start_chroot_script? In my tests I always end up with need_sudo_msg unless I modify the start_chroot_script.

Now to my results. It seems like there is no USER or SUDO_USER defined inside the docker image, unless you change make install to sudo make install inside start_chroot_script, but your BASE_USER will be gone then.

Running the following script inside the CustomPiOS docker (edit: with a custom module mentioned inside the next post):

echo ${USER}
echo ${BASE_USER}
echo ${SUDO_USER}

Gives following results without calling with sudo:

 
pi
 

And with sudo:

root

root

Note: Those empty lines aren't defined and not just wrong formatting.

So your modifications inside #142 and #155 both only account for the change to a sudo call inside the start_chroot_script, as otherwise it should end up in an empty SUDO_USER and failing here.
So you are right and there is definitely something wrong here, but your solutions wouldn't be the right way to go, if you really modified the start_chroot_script.
If you didn't modify start_chroot_script to sudo make install or sudo -u pi make install in your first tests, then we might need some test data from you too, as I couldn't reproduce your issue correctly.
We are already discussing internally what we have to change to fix it, without breaking anything else and maybe we find something more "elegant" than #155.

[mryel00]

Here my test setup to get the users.
I created and executed a new module called test with an empty config and following code inside:

#!/usr/bin/env bash

# Error handling
set -Ee

# shellcheck disable=SC1091
source /common.sh

echo_green "==================================="
echo "echo \${USER}" > /test.sh
echo "echo \${BASE_USER}" >> /test.sh
echo "echo \${SUDO_USER}" >> /test.sh
chmod +x /test.sh
/test.sh
sudo /test.sh
echo_green "==================================="

This will print out to green "lines" and between the results.
I then tested the same with a test branch of crowsnest with just outputing the users and no other changes. The results were the same with the non-sudo outputs.

[mitant]

There is a difference between directly echoing environment variables in Custompios's docker image versus echoing environment variables as part of the "build/package" process for custompios. When you call build in custompios, environment variables are injected, so they will no longer be empty.

SUDO_USER and USER I would call OS variables. SUDO_USER is empty if you are not sudoing as the user running the command.
BASE_USER is injected by the build/package process as is an expected variable by custompios, defaulting to "pi" unless overridden.

The only change I made to my crowsnest start_chroot_script that differs from yours is:

echo_green "Launch crowsnest install routine as ${BASE_USER}..."
sudo -u "${BASE_USER}" make installdocker

to make it work with my fork/PR.

If I use the original start_chroot_script where it is "sudo make install" , and I point at your master HEAD, I get the error I shared where it prints not_as_root_msg

[mryel00]

You are partly correct with the thing you wrote.
The thing here is that we don't echo inside the docker image but inside the "buildchain".
With pointing at my master HEAD you mean https://github.com/mryel00/crowsnest right?
If yes, there should be something printed above the not_as_root_msg, as I print there the actual variables the install script is using there. What's the output of those?
Like I said, if it doesn't work for you we need some more information, as it seems to differ somehow sometimes.

Also as a small note. The test module I provided is also the correct way to get the "buildchain" variables and even if it would be wrong. Those printed on my personal branch have to be the correct ones, as those shouldn't differ between echo and calling them inside an if statement.

[mryel00]

I just realized that you made a mistake.

If I use the original start_chroot_script where it is "sudo make install"

That's wrong the correct one would be make install. Have a look here

[mitant]

I was originally pointing at https://github.com/mainsail-crew/crowsnest/ not your specific fork
let me point at your fork, rerun the build, change it to make install and see what happens. make install is obv very different from sudo make install or sudo -u ${BASE_USER} make install so i guess we'll see what happens

[mitant]

Using your repo & using make install

...
�[92mLaunch crowsnest install routine...
�[0m
pi

�[31mcrowsnest�[0m - A webcam daemon for multiple Cams and stream services.
...

This worked.

I'm trying to think about what happened when I first tried this, with no changes. I only forked because I ran into a problem, otherwise I would not bother :). I think when I first tried to build, I ran into the "need_sudo_msg" which says please try sudo make install. Perhaps the pi user was not a sudoer yet?

I'm running another build, this time pointing at mainsail-crew/crowsnest instead of your fork just to confirm.

[mryel00]

Only my fork will work atm, as there is already the fix implemented 😄
I think the first time you run, it said that you are missing sudo privileges, so you just changed it to sudo make install. That lead to your PR then.
The fix is rather simple. Just change that line to if [[ -z "${SUDO_USER}" ]] && [[ "${CROWSNEST_UNATTENDED}" != "1" ]]; then
We still have to test this with a linux Docker setup, to confirm that it's working there too and then we will create a PR for that.

[mitant]

Cool! I'm happy to discard my PR, really didn't like 99% copy of the install script lol. I'm also glad that we were able to confirm that there's a legitimate issue there

[mitant]

Can also confirm the need root message against the mainsail-crew/crowsnest:HEAD . Now I know how I got to the mess I did :)
...
Processing triggers for libgdk-pixbuf-2.0-0:arm64 (2.42.2+dfsg-1+deb11u1) ...
Launch crowsnest install routine...

    You need to run this script with sudo priviledges!
    Please try 'sudo make install'

Exiting...
..

[KwadFan]

@mitant
Please let us know if the patch worked for you, released it today.
Regards Kwad

[mitant]

Works!