possible rm -rf / during screen installation

[Matszwe02]

Linux Distribution

rpiOS legacy 32bit on rpi3b

What happened

rm -rf /

What did you expect to happen

idk what should happen, raised an exception, install successful, either way, not rm -rf /

How to reproduce

• connect to freshly installed rpios
• git clone https://github.com/goodtft/LCD-show.git
• chmod -R 755 LCD-show
• cd LCD-show/
• sudo ./MPI3508-show
• [now reboot]
• sudo ./LCD35-show
• [now reboot]
• git clone https://github.com/dw-0/kiauh
• cd kiauh
• ./kiauh.sh
• 1 [install] 5 [klipperscreen] [as service] [X server]

Additional information

I don't have any logs, because, yeah...

[dw-0]

When choosing to install KlipperScreen with KIAUH, it's actually KlipperScreens own install script that gets executed by KIAUH.
I had a look at the script and the only line containing a rm -rf was this: https://github.com/KlipperScreen/KlipperScreen/blob/master/scripts/KlipperScreen-install.sh#L120
The referenced variable {KSENV} is defined here: https://github.com/KlipperScreen/KlipperScreen/blob/master/scripts/KlipperScreen-install.sh#L5

I am not sure if there is any constellation where the value of KSENV may be / at line 120 in the script.
@alfrix Any idea if and how this could have happened?

[alfrix]

When choosing to install KlipperScreen with KIAUH, it's actually KlipperScreens own install script that gets executed by KIAUH.

Before KIAUH runs KlipperScreen's installation script, KIAUH removes any existing KlipperScreen directory
[[ -d ${KLIPPERSCREEN_DIR} ]] && rm -rf "${KLIPPERSCREEN_DIR}"

kiauh/scripts/klipperscreen.sh

Line 50 in a929c69

[[ -d ${KLIPPERSCREEN_DIR} ]] && rm -rf "${KLIPPERSCREEN_DIR}"

leading into a first possible case of this unintended behavior if KLIPPERSCREEN_DIR is evaluated to "/" for some reason

then KS install script is executed
the install script will set:

KSENV="${KLIPPERSCREEN_VENV:-${HOME}/.KlipperScreen-env}"
...
    if [ -d $KSENV ]; then
        echo_text "Removing old virtual environment"
        rm -rf ${KSENV}
    fi

which means if KLIPPERSCREEN_VENV is set use that, and if that is evaluated into "/" you would have the same possible issue again

the else uses /home/username/.KlipperScreen-env, even if HOME is evaluated into nothing the tail /.KlipperScreen-env should ensure that nothing bad will happen with the default

It's worth noting that nor KLIPPERSCREEN_DIR or KLIPPERSCREEN_VENV are set by KlipperScreen or it's standalone installer, those are external variables for other installers

This leads me to the conclusion that every rm -rf should be double checked against "/" just in case, even if i find this scenario very unlikely

[dw-0]

Before KIAUH runs KlipperScreen's installation script, KIAUH removes any existing KlipperScreen directory [[ -d ${KLIPPERSCREEN_DIR} ]] && rm -rf "${KLIPPERSCREEN_DIR}"

kiauh/scripts/klipperscreen.sh

Line 50 in a929c69

[[ -d ${KLIPPERSCREEN_DIR} ]] && rm -rf "${KLIPPERSCREEN_DIR}"

leading into a first possible case of this unintended behavior if KLIPPERSCREEN_DIR is evaluated to "/" for some reason

Ah yes you are correct, there is a rm -rf before executing the KlipperScreen script. KLIPPERSCREEN_DIR is defined here: https://github.com/dw-0/kiauh/blob/master/scripts/globals.sh#L51

From my perspective that cannot be "/" in any case, nor can the KSENV from your script be "/".

No idea what happened there...

[Matszwe02]

I suspect that something might be off because I didn't have klipper installed, as the second time, that I installed ks after klipper, it worked.

I saw that it broke after or during installation of requirements for venv