Merge pull request #16678 from edsantiago/simplify_passthrough_env

CI setup: simplify environment passthrough code

contrib/cirrus/lib.sh

@@ -91,9 +91,21 @@ EPOCH_TEST_COMMIT="$CIRRUS_BASE_SHA"
 # testing operations on all platforms and versions.  This is necessary
 # to avoid needlessly passing through global/system values across
 # contexts, such as host->container or root->rootless user
-PASSTHROUGH_ENV_RE='(^CI.*)|(^CIRRUS)|(^DISTRO_NV)|(^GOPATH)|(^GOCACHE)|(^GOSRC)|(^SCRIPT_BASE)|(CGROUP_MANAGER)|(OCI_RUNTIME)|(^TEST.*)|(^PODBIN_NAME)|(^PRIV_NAME)|(^ALT_NAME)|(^ROOTLESS_USER)|(SKIP_USERNS)|(.*_NAME)|(.*_FQIN)|(NETWORK_BACKEND)|(DEST_BRANCH)'
+#
+# List of envariables which must be EXACT matches
+PASSTHROUGH_ENV_EXACT='CGROUP_MANAGER|DEST_BRANCH|DISTRO_NV|GOCACHE|GOPATH|GOSRC|NETWORK_BACKEND|OCI_RUNTIME|ROOTLESS_USER|SCRIPT_BASE|SKIP_USERNS'
+
+# List of envariable patterns which must match AT THE BEGINNING of the name.
+PASSTHROUGH_ENV_ATSTART='CI|TEST'
+
+# List of envariable patterns which can match ANYWHERE in the name
+PASSTHROUGH_ENV_ANYWHERE='_NAME|_FQIN'
+
+# Combine into one
+PASSTHROUGH_ENV_RE="(^($PASSTHROUGH_ENV_EXACT)\$)|(^($PASSTHROUGH_ENV_ATSTART))|($PASSTHROUGH_ENV_ANYWHERE)"
+
 # Unsafe env. vars for display
-SECRET_ENV_RE='(ACCOUNT)|(GC[EP]..+)|(SSH)|(PASSWORD)|(TOKEN)'
+SECRET_ENV_RE='ACCOUNT|GC[EP]..|SSH|PASSWORD|SECRET|TOKEN'
 
 # Type of filesystem used for cgroups
 CG_FS_TYPE="$(stat -f -c %T /sys/fs/cgroup)"
@@ -107,28 +119,18 @@ set +a
 lilto() { err_retry 8 1000 "" "$@"; }  # just over 4 minutes max
 bigto() { err_retry 7 5670 "" "$@"; }  # 12 minutes max
 
-# Print shell-escaped variable=value pairs, one per line, based on
-# variable name matching a regex.  This is intended to catch
-# variables being passed down from higher layers, like Cirrus-CI.
+# Return a list of environment variables that should be passed through
+# to lower levels (tests in containers, or via ssh to rootless).
+# We return the variable names only, not their values. It is up to our
+# caller to reference values.
 passthrough_envars(){
-    local xchars
     local envname
-    local envval
-    # Avoid values containing entirely punctuation|control|whitespace
-    xchars='[:punct:][:cntrl:][:space:]'
     warn "Will pass env. vars. matching the following regex:
     $PASSTHROUGH_ENV_RE"
-    for envname in $(awk 'BEGIN{for(v in ENVIRON) print v}' | \
-                         grep -Ev "SETUP_ENVIRONMENT" | \
-                         grep -Ev "$SECRET_ENV_RE" | \
-                         grep -E "$PASSTHROUGH_ENV_RE"); do
-
-            envval="${!envname}"
-            [[ -n $(tr -d "$xchars" <<<"$envval") ]] || continue
-
-            # Properly escape values to prevent injection
-            printf -- "$envname=%q\n" "$envval"
-    done
+    compgen -A variable | \
+        grep -Ev "SETUP_ENVIRONMENT" | \
+        grep -Ev "$SECRET_ENV_RE" | \
+        grep -E  "$PASSTHROUGH_ENV_RE"
 }
 
 setup_rootless() {

contrib/cirrus/lib.sh.t

@@ -0,0 +1,193 @@
+#!/bin/bash
+#
+# tests for lib.sh
+#
+
+# To ensure consistent sorting
+export LC_ALL=C
+
+###############################################################################
+# BEGIN code to define a clean safe environment
+
+# Envariables which we should keep; anything else, we toss.
+declare -a keep_env_list=(IFS HOME PATH SECRET_ENV_RE
+                          PASSTHROUGH_ENV_EXACT
+                          PASSTHROUGH_ENV_ATSTART
+                          PASSTHROUGH_ENV_ANYWHERE
+                          PASSTHROUGH_ENV_RE
+                          TMPDIR tmpdir keep_env rc_file testnum_file)
+declare -A keep_env
+for i in "${keep_env_list[@]}"; do
+    keep_env[$i]=1
+done
+
+# END   code to define a clean safe environment
+###############################################################################
+# BEGIN test scaffolding
+
+tmpdir=$(mktemp --tmpdir --directory lib-sh-tests.XXXXXXX)
+# shellcheck disable=SC2154
+trap 'status=$?; rm -rf $tmpdir;exit $status' 0
+
+# Needed by lib.sh, but we don't actually need anything in it
+touch "$tmpdir"/common_lib.sh
+
+# Iterator and return code. Because some tests run in subshells (to avoid
+# namespace pollution), variables aren't preserved. Use files to track them.
+testnum_file=$tmpdir/testnum
+rc_file=$tmpdir/rc
+
+echo 0 >"$testnum_file"
+echo 0 >"$rc_file"
+
+# Helper function: runs passthrough_envars(), compares against expectations
+function check_passthrough {
+    testnum=$(< "$testnum_file")
+    testnum=$((testnum + 1))
+    echo $testnum > "$testnum_file"
+
+    # shellcheck disable=SC2046,SC2005,SC2116
+    actual="$(echo $(passthrough_envars))"
+
+    if [[ "$actual" = "$1" ]]; then
+        # Multi-level echo flattens newlines, makes success messages readable
+        # shellcheck disable=SC2046,SC2005,SC2116
+        echo $(echo "ok $testnum $2")
+    else
+        echo "not ok $testnum $2"
+        echo "#  expected: $1"
+        echo "#    actual: $actual"
+        echo 1 >| "$rc_file"
+    fi
+}
+
+# END   test scaffolding
+###############################################################################
+
+# vars and a function needed by lib.sh
+# shellcheck disable=SC2034
+{
+    AUTOMATION_LIB_PATH=$tmpdir
+    CIRRUS_BASE_SHA=x
+    CIRRUS_TAG=x
+    function warn() {
+        :
+    }
+    # shellcheck disable=all
+    source $(dirname "$0")/lib.sh
+}
+
+# Our environment is now super-polluted. Clean it up, preserving critical env.
+while read -r v;do
+      if [[ -z "${keep_env[$v]}" ]]; then
+          unset "$v" 2>/dev/null
+      fi
+done < <(compgen -A variable)
+
+# begin actual tests
+
+check_passthrough "" "with empty environment"
+
+#
+# Now set all sorts of secrets, which should be excluded
+#
+# shellcheck disable=SC2034
+{
+    ACCOUNT_ABC=1
+    ABC_ACCOUNT=1
+    ABC_ACCOUNT_DEF=1
+    GCEFOO=1
+    GCPBAR=1
+    SSH123=1
+    NOTSSH=1
+    SSH=1
+    PASSWORD=1
+    MYSECRET=1
+    SECRET2=1
+    TOKEN=1
+    check_passthrough "" "secrets are filtered"
+}
+
+# These are passed through only when they match EXACTLY.
+readarray -d '|' -t pt_exact <<<"$PASSTHROUGH_ENV_EXACT"
+# shellcheck disable=SC2048
+for s in ${pt_exact[*]}; do
+    # Run inside a subshell, to avoid cluttering environment
+    (
+        eval "${s}=1"             # This is the only one that should be passed
+        eval "a${s}=1"
+        eval "${s}z=1"
+        eval "YYY_${s}_YYY=1"
+        eval "ZZZ_${s}=1"
+        eval "${s}_ZZZ=1"
+
+        # Only the exact match should be passed
+        check_passthrough "$s" "exact match only: $s"
+    )
+done
+
+# These are passed through only when they match AT THE BEGINNING.
+#
+# Also, we run this _entire_ test inside a subshell, cluttering the
+# environment, so we're testing that passthrough_envars can handle
+# and return long lists of unrelated matches. Kind of a pointless
+# test, there's not really any imaginable way that could fail.
+(
+    # Inside the subshell. Start with null expectations.
+    expect=
+
+    # WARNING! $PASSTHROUGH_ENV_ATSTART must be in alphabetical order,
+    # because passthrough_envars always returns a sorted list and (see
+    # subshell comments above) we're incrementally adding to our env.
+    readarray -d '|' -t pt_atstart <<<"$PASSTHROUGH_ENV_ATSTART"
+    # shellcheck disable=SC2048
+    for s in ${pt_atstart[*]}; do
+        eval "${s}=1"
+        eval "${s}123=1"
+        eval "NOPE_${s}=1"
+        eval "NOR_${s}_EITHER=1"
+
+        if [[ -n "$expect" ]]; then
+            expect+=" "
+        fi
+        expect+="$s ${s}123"
+
+        check_passthrough "$expect" "at start only: $s"
+    done
+)
+
+# These are passed through if they match ANYWHERE IN THE NAME
+readarray -d '|' -t pt_anywhere <<<"$PASSTHROUGH_ENV_ANYWHERE"
+# shellcheck disable=SC2048
+for s in ${pt_anywhere[*]}; do
+    (
+        eval "${s}=1"
+        eval "${s}z=1"
+        eval "z${s}=1"
+        eval "z${s}z=1"
+
+        check_passthrough "${s} ${s}z z${s} z${s}z" "anywhere: $s"
+    )
+done
+
+# And, to guard against null runs of the above loops, hardcoded tests of each:
+# shellcheck disable=SC2034
+(
+    CI=1
+    CI_FOO=1
+    CIRRUS_BAR=1
+    GOPATH=gopath
+    GOPATH_NOT=not
+    ROOTLESS_USER=rootless
+    ZZZ_NAME=1
+
+    check_passthrough "CI CIRRUS_BAR CI_FOO GOPATH ROOTLESS_USER ZZZ_NAME" \
+                      "final handcrafted sanity check"
+)
+
+# Final check
+check_passthrough "" "Environment remains unpolluted at end"
+
+# Done
+# shellcheck disable=all
+exit $(<"$rc_file")

contrib/cirrus/prebuild.sh

@@ -42,6 +42,9 @@ if [[ "${DISTRO_NV}" =~ fedora ]]; then
         ./.github/actions/check_cirrus_cron/* \
         hack/get_ci_vm.sh
 
+    # Tests for lib.sh
+    showrun ${SCRIPT_BASE}/lib.sh.t
+
     export PREBUILD=1
     showrun bash ${CIRRUS_WORKING_DIR}/.github/actions/check_cirrus_cron/test.sh
 fi

contrib/cirrus/runner.sh

@@ -136,11 +136,11 @@ exec_container() {
 
     # Line-separated arguments which include shell-escaped special characters
     declare -a envargs
-    while read -r var_val; do
+    while read -r var; do
         # Pass "-e VAR" on the command line, not "-e VAR=value". Podman can
         # do a much better job of transmitting the value than we can,
         # especially when value includes spaces.
-        envargs+=("-e" "$(awk -F= '{print $1}' <<<$var_val)")
+        envargs+=("-e" "$var")
     done <<<"$(passthrough_envars)"
 
     # VM Images and Container images are built using (nearly) identical operations.

contrib/cirrus/setup_environment.sh

@@ -51,8 +51,8 @@ git config --system --add safe.directory $GOSRC
 echo -e "\n# Begin single-use VM global variables (${BASH_SOURCE[0]})" \
     > "/etc/ci_environment"
 (
-    while read -r env_var_val; do
-        echo "$env_var_val"
+    while read -r env_var; do
+        printf -- "%s=%q\n" "${env_var}" "${!env_var}"
     done <<<"$(passthrough_envars)"
 ) >> "/etc/ci_environment"