diff --git a/bin/dib-lint b/bin/dib-lint index 248778499..bedf1da3e 100755 --- a/bin/dib-lint +++ b/bin/dib-lint @@ -54,7 +54,7 @@ excluded() { } error() { - echo "ERROR: $1" + echo -e "ERROR: $1" rc=1 } @@ -146,6 +146,32 @@ for i in $(find elements -type f \ fi fi fi + + + # check that sudo calls in phases run outside the chroot look + # "safe"; meaning that they seem to operate within the chroot + # somehow. This is not fool-proof, but catches egregious errors, + # and makes you think about it if you're doing something outside + # the box. + if ! excluded safe_sudo; then + if [[ $(dirname $i) =~ (root.d|extra-data.d|block-device.d|finalise.d|cleanup.d) ]]; then + while read LINE + do + if [[ $LINE =~ "sudo " ]]; then + # messy regex ahead! Don't match: + # - explicitly ignored + # - basic comments + # - install-packages ... sudo ... + # - any of the paths passed into the out-of-chroot elements + if [[ $LINE =~ (dib-lint: safe_sudo|^#|install-packages|TARGET_ROOT|IMAGE_BLOCK_DEVICE|TMP_MOUNT_PATH|TMP_IMAGE_PATH) ]]; then + continue + fi + error "$i : potentially unsafe sudo\n -- $LINE" + fi + done < $i + fi + fi + done for i in $(find elements -type f -and -name '*.rst' -or -type f -executable); do diff --git a/doc/source/developer/developing_elements.rst b/doc/source/developer/developing_elements.rst index a999a22db..0cee5946e 100644 --- a/doc/source/developer/developing_elements.rst +++ b/doc/source/developer/developing_elements.rst @@ -425,3 +425,30 @@ example if one were building tripleo-images, the variable would be set like: export ELEMENTS_PATH=tripleo-image-elements/elements disk-image-create rhel7 cinder-api + +Linting +------- + +You should always run ``bin/dib-lint`` over your elements. It will +warn you of common issues. + +sudo +"""" + +Using ``sudo`` outside the chroot environment can cause breakout +issues where you accidentally modify parts of the host +system. ``dib-lint`` will warn if it sees ``sudo`` calls that do not +use the path arguments given to elements running outside the chroot. + +To disable the error for a call you know is safe, add + +:: + + # dib-lint: safe_sudo + +to the end of the ``sudo`` command line. To disable the check for an +entire file, add + +:: + + # dib-lint: disable=safe_sudo diff --git a/elements/apt-sources/extra-data.d/99-override-default-apt-sources b/elements/apt-sources/extra-data.d/99-override-default-apt-sources index 7381693b8..94fa6e9b2 100755 --- a/elements/apt-sources/extra-data.d/99-override-default-apt-sources +++ b/elements/apt-sources/extra-data.d/99-override-default-apt-sources @@ -21,5 +21,5 @@ DIB_APT_SOURCES=`readlink -f $DIB_APT_SOURCES` # copy the sources.list to cloudimg pushd $TMP_MOUNT_PATH/etc/apt/ -sudo cp -f $DIB_APT_SOURCES sources.list +sudo cp -f $DIB_APT_SOURCES sources.list # dib-lint: safe_sudo popd diff --git a/elements/baremetal/cleanup.d/99-extract-kernel-and-ramdisk b/elements/baremetal/cleanup.d/99-extract-kernel-and-ramdisk index 7c4a68571..d1667ee6d 100755 --- a/elements/baremetal/cleanup.d/99-extract-kernel-and-ramdisk +++ b/elements/baremetal/cleanup.d/99-extract-kernel-and-ramdisk @@ -14,6 +14,8 @@ # License for the specific language governing permissions and limitations # under the License. +# dib-lint: disable=safe_sudo + if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then set -x fi diff --git a/elements/bootloader/cleanup.d/51-bootloader b/elements/bootloader/cleanup.d/51-bootloader index 40a3b1ad4..28202f8bf 100755 --- a/elements/bootloader/cleanup.d/51-bootloader +++ b/elements/bootloader/cleanup.d/51-bootloader @@ -15,6 +15,8 @@ # License for the specific language governing permissions and limitations # under the License. +# dib-lint: disable=safe_sudo + if [ ${DIB_DEBUG_TRACE:-1} -gt 0 ]; then set -x fi diff --git a/elements/dpkg/extra-data.d/01-copy-apt-keys b/elements/dpkg/extra-data.d/01-copy-apt-keys index 668135c7f..52f4ff790 100755 --- a/elements/dpkg/extra-data.d/01-copy-apt-keys +++ b/elements/dpkg/extra-data.d/01-copy-apt-keys @@ -31,9 +31,9 @@ if [ -e ${DIR} ]; then echo "${DIR} already exists!" exit 1 fi -sudo mkdir -p ${DIR} +sudo mkdir -p ${DIR} # dib-lint: safe_sudo # Copy to DIR for KEY in $(find ${DIB_ADD_APT_KEYS} -type f); do - sudo cp -L ${KEY} ${DIR} + sudo cp -L ${KEY} ${DIR} # dib-lint: safe_sudo done diff --git a/elements/ironic-agent/cleanup.d/99-ramdisk-create b/elements/ironic-agent/cleanup.d/99-ramdisk-create index 9a185da17..d64332dc5 100755 --- a/elements/ironic-agent/cleanup.d/99-ramdisk-create +++ b/elements/ironic-agent/cleanup.d/99-ramdisk-create @@ -1,5 +1,7 @@ #!/bin/bash +# dib-lint: disable=safe_sudo + if [ "${DIB_DEBUG_TRACE:-0}" -gt 0 ]; then set -x fi diff --git a/elements/iso/cleanup.d/100-build-iso b/elements/iso/cleanup.d/100-build-iso index 5430a0831..4832c9444 100755 --- a/elements/iso/cleanup.d/100-build-iso +++ b/elements/iso/cleanup.d/100-build-iso @@ -14,6 +14,8 @@ # License for the specific language governing permissions and limitations # under the License. +# dib-lint: disable=safe_sudo + if [ ${DIB_DEBUG_TRACE:-1} -gt 0 ]; then set -x fi diff --git a/elements/partitioning-sfdisk/block-device.d/10-partitioning-sfdisk b/elements/partitioning-sfdisk/block-device.d/10-partitioning-sfdisk index 447f23a0e..a529e0a47 100755 --- a/elements/partitioning-sfdisk/block-device.d/10-partitioning-sfdisk +++ b/elements/partitioning-sfdisk/block-device.d/10-partitioning-sfdisk @@ -1,5 +1,7 @@ #!/bin/bash +# dib-lint: disable=safe_sudo + if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then set -x fi diff --git a/elements/pypi/extra-data.d/00-mount-pypi-mirror b/elements/pypi/extra-data.d/00-mount-pypi-mirror index 7d2923f4e..570a7711a 100755 --- a/elements/pypi/extra-data.d/00-mount-pypi-mirror +++ b/elements/pypi/extra-data.d/00-mount-pypi-mirror @@ -10,6 +10,6 @@ MIRROR_SOURCE=$DIB_IMAGE_CACHE/pypi/mirror/ if [ -d "$MIRROR_SOURCE" ]; then MIRROR_TARGET=$TMP_MOUNT_PATH/tmp/pypi - sudo mkdir -p $MIRROR_SOURCE $MIRROR_TARGET - sudo mount --bind $MIRROR_SOURCE $MIRROR_TARGET + sudo mkdir -p $MIRROR_SOURCE $MIRROR_TARGET # dib-lint: safe_sudo + sudo mount --bind $MIRROR_SOURCE $MIRROR_TARGET # dib-lint: safe_sudo fi diff --git a/elements/rhel/root.d/10-rhel-cloud-image b/elements/rhel/root.d/10-rhel-cloud-image index 1571249fd..909949da9 100755 --- a/elements/rhel/root.d/10-rhel-cloud-image +++ b/elements/rhel/root.d/10-rhel-cloud-image @@ -1,5 +1,7 @@ #!/bin/bash +# dib-lint: disable=safe_sudo + if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then set -x fi diff --git a/elements/source-repositories/extra-data.d/98-source-repositories b/elements/source-repositories/extra-data.d/98-source-repositories index 4770bea60..76d874ad2 100755 --- a/elements/source-repositories/extra-data.d/98-source-repositories +++ b/elements/source-repositories/extra-data.d/98-source-repositories @@ -1,5 +1,6 @@ #!/bin/bash +# dib-lint: disable=safe_sudo if [ ${DIB_DEBUG_TRACE:-0} -gt 1 ]; then set -x fi diff --git a/elements/ubuntu-core/root.d/10-cache-ubuntu-image b/elements/ubuntu-core/root.d/10-cache-ubuntu-image index 550fdc2e8..10faddfa1 100755 --- a/elements/ubuntu-core/root.d/10-cache-ubuntu-image +++ b/elements/ubuntu-core/root.d/10-cache-ubuntu-image @@ -1,6 +1,8 @@ #!/bin/bash # These are useful, or at worst not harmful, for all images we build. +# dib-lint: disable=safe_sudo + if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then set -x fi diff --git a/elements/vm/block-device.d/10-partition b/elements/vm/block-device.d/10-partition index df0732edd..e1bc8ddff 100755 --- a/elements/vm/block-device.d/10-partition +++ b/elements/vm/block-device.d/10-partition @@ -1,5 +1,7 @@ #!/bin/bash +# dib-lint: disable=safe_sudo + if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then set -x fi diff --git a/elements/yum-minimal/root.d/08-yum-chroot b/elements/yum-minimal/root.d/08-yum-chroot index 2d0850814..6cf0dfaf2 100755 --- a/elements/yum-minimal/root.d/08-yum-chroot +++ b/elements/yum-minimal/root.d/08-yum-chroot @@ -14,6 +14,9 @@ # License for the specific language governing permissions and limitations # under the License. # + +# dib-lint: disable=safe_sudo + if [ "${DIB_DEBUG_TRACE:-0}" -gt 0 ]; then set -x fi