Implement clean_chroot to avoid host env variables like TMP to leak into the chroot #280

mika · 2024-08-14T14:47:05Z

Some implementation notes:

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

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.

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.

FTR, 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 we'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.

Closes: #232

…nto the chroot Some implementation notes: If we use `env -i`, then we can no longer export shell functions. So export -f "error_handler" had to be removed. `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. 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. FTR, 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 we'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. Closes: #232

mika · 2024-08-14T14:49:56Z

This is the result of squashing the commits from #267 and extending the commit message accordingly.

What I'm not entirely sure about yet is the usage of calls like:

clean_chroot "$MNTPOINT" DEBIAN_FRONTEND=$DEBIAN_FRONTEND apt-get [...]

@adrelanos are you sure this behaves as expected? 🤔
Would it make sense to support DEBIAN_FRONTEND via the additional_vars you implemented in clean_chroot()? 🤔

github-actions

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

shellcheck

📝 [shellcheck] _{reported by reviewdog 🐶}
Double quote to prevent globbing and word splitting. SC2086

                       fi
                     done
-                    [ -x "$MNTPOINT"/bin/umount ] && chroot "$MNTPOINT" umount -a >/dev/null 2>&1 || true
+                    [ -x "$MNTPOINT"/bin/umount ] && clean_chroot "$MNTPOINT" umount -a >/dev/null 2>&1 || true

                     # ugly, but make sure we really don't leave anything (/proc /proc and
                     # /dev /dev are intended, trying to work around timing issues, see #657023)
                     for ARG in /run/udev /sys /proc /proc /dev/pts /dev/pts /dev /dev ; do
-                      [ -x "$MNTPOINT"/bin/umount ] && chroot "$MNTPOINT" umount $ARG >/dev/null 2>&1 || true
+                      [ -x "$MNTPOINT"/bin/umount ] && clean_chroot "$MNTPOINT" umount $ARG >/dev/null 2>&1 || true

	[ -d "${CONFFILES}"/bin ] && cp $VERBOSE -a -L "${CONFFILES}"/bin/* "${MNTPOINT}"/bin/
	[ -d "${CONFFILES}"/boot ] && cp $VERBOSE -a -L "${CONFFILES}"/boot/* "${MNTPOINT}"/boot/
	[ -d "${CONFFILES}"/etc ] && cp $VERBOSE -a -L "${CONFFILES}"/etc/* "${MNTPOINT}"/etc/
	[ -d "${CONFFILES}"/sbin ] && cp $VERBOSE -a -L "${CONFFILES}"/sbin/* "${MNTPOINT}"/sbin/
	[ -d "${CONFFILES}"/share ] && cp $VERBOSE -a -L "${CONFFILES}"/share/* "${MNTPOINT}"/share/
	[ -d "${CONFFILES}"/usr ] && cp $VERBOSE -a -L "${CONFFILES}"/usr/* "${MNTPOINT}"/usr/
	[ -d "${CONFFILES}"/var ] && cp $VERBOSE -a -L "${CONFFILES}"/var/* "${MNTPOINT}"/var/

Implement clean_chroot to avoid host env variables like TMP to leak into the chroot #280

Are you sure you want to change the base?

Implement clean_chroot to avoid host env variables like TMP to leak into the chroot #280

Conversation

mika commented Aug 14, 2024

mika commented Aug 14, 2024

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot Aug 14, 2024

Choose a reason for hiding this comment