Merge "Add a best-effort sudo safety check"

bin/dib-lint

@@ -54,7 +54,7 @@ excluded() {
 }
 
 error() {
-    echo "ERROR: $1"
+    echo -e "ERROR: $1"
     rc=1
 }
 
@@ -150,6 +150,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
 
 echo "Checking indents..."

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

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

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

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

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

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

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

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

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

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

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

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

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

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