grml / grml-debootstrap

wrapper around debootstrap
59 stars 27 forks source link

implement clean_chroot, #267

Open adrelanos opened 8 months ago

adrelanos commented 8 months ago

avoid host environment variables such as TMP to leak into the chroot

fixes https://github.com/grml/grml-debootstrap/issues/232

adrelanos commented 8 months ago

Some optional explanation why I made certain changes the way I made them.

If we use env -i then we can no longer export shell functions. So export -f "error_handler" had to be removed.

clean_chroot function has become more complex than we might have hoped for.

Even PATH needs to be set. Otherwise clean_chroot "$MNTPOINT" grub-install would fail because grub-install is in /usr/sbin/grub-install in the chroot.

A potential alternative that I personally would use is sudo as that already has sane environment scrubbing but I wasn't sure you would want a dependency on sudo. Also maybe in the future you would want to run most of grml-deboostrap as a unprivileged user and only use a privilege escalation tool (such as sudo) when needed and/or run commands unprivileged inside the chroot.

http_proxy has to be passed otherwise apt-cacher-ng would be broken by this commit. While at it, I completed it and added https_proxy, and ALL_PROXY there too for completeness sake.

Which environment variables are passed into the chroot is currently hardcoded. I didn't think it is important to be able to configure that. If somebody needs to configure that, I guess they can create a feature request and/or send a pull request.

The xtrace (bash -x) could be minimized if the code to set the environment variables was just run and by using global variables. That however would come at the cost at slightly more code complexity.

I was also wondering if it was better to use a similar mechanism to the one you're using for CHROOT_VARIABLES, but that would not work because only the chroot-script reads those. But you're not only using that but also other calls from grml-debootstrap to chroot (now clean_chroot) so the environment variables need to be set at the grml-debootstrap level.

adrelanos commented 7 months ago

Resolved merge conflict.

adrelanos commented 4 months ago

Ping?

adrelanos commented 4 months ago

Good now?

adrelanos commented 3 months ago

Ping?

adrelanos commented 2 months ago

Ping?

mika commented 4 weeks ago

Ping?

Sorry4delay, finally found some time to look into this, see https://github.com/grml/grml-debootstrap/pull/280