Fix cci-export mangling the values of multiline strings (e.g., cert…

…ificates) (#102) * X-Smart-Branch-Parent: master * Test: Reproduce the problem from the broken v0.3.22 * Implement fix for the problem Co-authored-by: msugakov <[email protected]>
stackrox · Jan 12, 2022 · dad5109 · dad5109
1 parent 22a63f8
commit dad5109
Show file tree

Hide file tree

Showing 5 changed files with 132 additions and 31 deletions.
diff --git a/images/static-contents/bin/bash-wrapper b/images/static-contents/bin/bash-wrapper
@@ -20,19 +20,40 @@ cci-export() {
       return 1
     fi
 
-    # Use export with default value: 'export FOO="${FOO:-bar}"', so that variables set in the environment are not overwritten by `$BASH_ENV`
-    key_part="export ${key}"
-    # shellcheck disable=SC2016 # we must produce literal ${} symbols
-    value_part="$(printf '"${%q:-"%q"}"\n' "$key" "$value")"
-
-    # Remove all export-lines for the same exported variable, to 'forget' about past cci-export calls,
+    # Use export with default value in the following form
+    # export __CCI_EXPORT_FOO_VALUE=bar
+    # export FOO="${FOO:-"${__CCI_EXPORT_FOO_VALUE}"}"'
+    # for the following reasons:
+    # - Using Bash default value (FOO=${FOO:-"${__CCI_EXPORT_FOO_VALUE}"})  - so that variables already set in the environment are not overwritten by `$BASH_ENV`
+    # - Using a variable holding the default value (__CCI_EXPORT_FOO_VALUE=value) - so that multiline values (e.g., certificates) are correctly escaped
+    #
+    # An example of BASH_ENV contents:
+    # export __CCI_EXPORT_FOO_VALUE=bar
+    # export FOO="${FOO:-"${__CCI_EXPORT_FOO_VALUE}"}"'
+    # export __CCI_EXPORT_BAZ_VALUE=baz
+    # export BAZ="${BAZ:-"${__CCI_EXPORT_BAZ_VALUE}"}"'
+
+    shadow_key="__CCI_EXPORT_${key}_VALUE"
+
+    # Remove all lines starting with:
+    # export __CCI_EXPORT_VAR_VALUE=
+    # export VAR=
+    # for the same exported variable (VAR), to 'forget' about past cci-export calls,
     # otherwise the first call to cci-export would define a default value for the variable, so that
     # second and subsequent calls to cci-export would have no effect.
     if [[ -f "$BASH_ENV" ]]; then
       filtered_envfile="$(mktemp -t "bash.env-XXXX")"
-      grep --invert-match --fixed-strings "${key_part}=" "$BASH_ENV" > "${filtered_envfile}" && mv "${filtered_envfile}" "$BASH_ENV"
+      # The first pattern (-e) is necessary for correctness
+      # The second pattern is optional for correctness, but it prevents duplicate lines cluttering the file
+      grep --invert-match --fixed-strings \
+        -e "export ${shadow_key}=" \
+        -e "export ${key}=" \
+        "$BASH_ENV" > "${filtered_envfile}" && mv "${filtered_envfile}" "$BASH_ENV"
     fi
-    echo "${key_part}=${value_part}" >> "$BASH_ENV"
+
+    printf "export %s=%q\n" "$shadow_key" "$value" >> "$BASH_ENV"
+    # shellcheck disable=SC2016 # we must produce literal ${} symbols
+    printf 'export %s="${%s:-"${%s}"}"\n' "$key" "$key" "$shadow_key" >> "$BASH_ENV"
   fi
 }
 

diff --git a/images/test/bats/cci-export.bats b/images/test/bats/cci-export.bats
@@ -7,7 +7,12 @@ bats_helpers_root="/usr/lib/node_modules"
 load "${bats_helpers_root}/bats-support/load.bash"
 load "${bats_helpers_root}/bats-assert/load.bash"
 
+foo_printer() {
+  "$HOME/test/bats/foo-printer.sh" "${@}"
+}
+
 setup() {
+  export _CERT="$HOME/test/bats/test-ca.crt"
   export _FILE="$HOME/test/bats/FILE"
   # Create a file used in test-cases using subshell execution of 'cat'
   echo "1.2.3" > "${_FILE}"
@@ -23,7 +28,7 @@ setup() {
   assert_output "$bash_env"
   run cat $BASH_ENV
   assert_output ""
-  run "$HOME/test/foo-printer.sh"
+  run foo_printer
   assert_output "FOO: "
   run test -n $CIRCLECI
   assert_success
@@ -38,33 +43,74 @@ setup() {
 
   run cci-export FOO cci1
   assert_success
-  run "$HOME/test/foo-printer.sh"
+  run foo_printer
   assert_output "FOO: cci1"
   refute_output "FOO: "
 }
 
 @test "cci-export sanity check single value" {
   run cci-export FOO cci1
   assert_success
-  run "$HOME/test/foo-printer.sh"
+  run foo_printer
   assert_output "FOO: cci1"
   refute_output "FOO: "
 
   run cci-export FOO cci2
   assert_success
-  run "$HOME/test/foo-printer.sh"
+  run foo_printer
   assert_output "FOO: cci2"
   refute_output "FOO: cci1"
 }
 
 @test "cci-export should escape special characters in values" {
   run cci-export FOO 'quay.io/rhacs-"eng"/super $canner:2.21.0-15-{{g44}(8f)2dc8fa}'
   assert_success
-  run "$HOME/test/foo-printer.sh"
+  run foo_printer
   assert_output 'FOO: quay.io/rhacs-"eng"/super $canner:2.21.0-15-{{g44}(8f)2dc8fa}'
   refute_output "FOO: "
 }
 
+@test "cci-export should properly handle multiline values" {
+  # Sanity check on cert test fixture
+  run test -f "${_CERT}"
+  assert_success
+  # The unprocessed cert should be parsable with openssl
+  run openssl x509 -in "${_CERT}" -noout
+  assert_success
+
+  run cci-export CERT "$(cat ${_CERT})"
+  assert_success
+
+  post_cert="${_CERT}.post"
+  foo_printer CERT --silent > "$post_cert"
+  # openssl should be able to load the cert after processing it with cci-export
+  run openssl x509 -in "$post_cert" -noout
+  assert_success
+
+  run diff -q "${_CERT}" "$post_cert"
+  assert_success
+}
+
+@test "cci-export should allow overwriting multiline values" {
+  run cci-export CERT "$(cat ${_CERT})"
+  assert_success
+  run foo_printer "CERT"
+  assert_line "CERT: -----BEGIN CERTIFICATE-----"
+  assert_line "-----END CERTIFICATE-----"
+
+  run cci-export CERT "dummy"
+  run foo_printer "CERT"
+  assert_output "CERT: dummy"
+}
+
+@test "cci-export should not leave duplicate lines in BASH_ENV" {
+  run cci-export FOO foo # creates 2 lines in BASH_ENV
+  run cci-export FOO foo2 # removes 2 and creates 2 lines in BASH_ENV
+
+  run bash -c "grep FOO "$BASH_ENV" | wc -l"
+  assert_output 2
+}
+
 @test "cci-export sanity check many values" {
   run cat "${_FILE}"
   assert_output "1.2.3"
@@ -74,15 +120,15 @@ setup() {
   run cci-export VAR2 "text/$VAR/text:$(cat "${_FILE}")"
   run cci-export IMAGE3 "text/$VAR/text:$(cat "${_FILE}")"
 
-  run "$HOME/test/foo-printer.sh" "VAR1"
+  run foo_printer "VAR1"
   assert_output "VAR1: text/$VAR/text:$(cat "${_FILE}")"
   assert_output "VAR1: text/placeholder/text:1.2.3"
 
-  run "$HOME/test/foo-printer.sh" VAR2
+  run foo_printer VAR2
   assert_output "VAR2: text/$VAR/text:$(cat "${_FILE}")"
   assert_output "VAR2: text/placeholder/text:1.2.3"
 
-  run "$HOME/test/foo-printer.sh" IMAGE3
+  run foo_printer IMAGE3
   assert_output "IMAGE3: text/$VAR/text:$(cat "${_FILE}")"
   assert_output "IMAGE3: text/placeholder/text:1.2.3"
 }
@@ -92,38 +138,38 @@ setup() {
   run cci-export PART1_PART2 "value_joined"
   run cci-export PART1 "value2"
 
-  run "$HOME/test/foo-printer.sh" PART1
+  run foo_printer PART1
   assert_output "PART1: value2"
   refute_output "PART1: value1"
-  run "$HOME/test/foo-printer.sh" PART1_PART2
+  run foo_printer PART1_PART2
   assert_output "PART1_PART2: value_joined"
 }
 
 @test "exported variable should be respected in a script" {
   export FOO=bar
-  run "$HOME/test/foo-printer.sh"
+  run foo_printer
   assert_output "FOO: bar"
   refute_output "FOO: "
 }
 
 @test "shadowed variable should be respected in a script" {
-  FOO=bar run "$HOME/test/foo-printer.sh"
+  FOO=bar run foo_printer
   assert_output "FOO: bar"
   refute_output "FOO: "
 }
 
 @test "exported variable should have priority over the cci-exported one" {
   run cci-export FOO cci
   export FOO=bar
-  run "$HOME/test/foo-printer.sh"
+  run foo_printer
   assert_output "FOO: bar"
   refute_output "FOO: cci"
   refute_output "FOO: "
 }
 
 @test "shadowed variable should have priority over the cci-exported one" {
   run cci-export FOO cci
-  FOO=bar run "$HOME/test/foo-printer.sh"
+  FOO=bar run foo_printer
   assert_output "FOO: bar"
   refute_output "FOO: cci"
   refute_output "FOO: "
@@ -132,7 +178,7 @@ setup() {
 @test "shadowed variable should have priority over both: the exported and the cci-exported one" {
   export FOO=bar-export
   run cci-export FOO cci
-  FOO=bar-shadow run "$HOME/test/foo-printer.sh"
+  FOO=bar-shadow run foo_printer
   assert_output "FOO: bar-shadow"
   refute_output "FOO: bar-export"
   refute_output "FOO: cci"
@@ -141,7 +187,7 @@ setup() {
 
   run cci-export FOO cci2
   export FOO=bar-export2
-  FOO=bar-shadow2 run "$HOME/test/foo-printer.sh"
+  FOO=bar-shadow2 run foo_printer
   assert_output "FOO: bar-shadow2"
   refute_output "FOO: bar-export2"
   refute_output "FOO: cci2"

diff --git a/images/test/bats/foo-printer.sh b/images/test/bats/foo-printer.sh
@@ -0,0 +1,23 @@
+#!/usr/bin/env bash
+# prints the name (unless -s|--silent) and value of the variable given as parameter
+
+set -eo pipefail
+
+key="FOO"
+value="$FOO"
+sflag=0
+
+while [[ "$#" -gt 0 ]]; do
+  case $1 in
+    -s|--silent) sflag=1
+        ;;
+    *)  key="$1"; value="${!1}"
+        ;;
+  esac
+  shift
+done
+
+if (( sflag == 0 )); then
+  echo -n "$key: "
+fi
+echo "$value"
diff --git a/images/test/bats/test-ca.crt b/images/test/bats/test-ca.crt
@@ -0,0 +1,18 @@
+-----BEGIN CERTIFICATE-----
+MIIC2jCCAkMCAg38MA0GCSqGSIb3DQEBBQUAMIGbMQswCQYDVQQGEwJKUDEOMAwG
+A1UECBMFVG9reW8xEDAOBgNVBAcTB0NodW8ta3UxETAPBgNVBAoTCEZyYW5rNERE
+MRgwFgYDVQQLEw9XZWJDZXJ0IFN1cHBvcnQxGDAWBgNVBAMTD0ZyYW5rNEREIFdl
+YiBDQTEjMCEGCSqGSIb3DQEJARYUc3VwcG9ydEBmcmFuazRkZC5jb20wHhcNMTIw
+ODIyMDUyNzQxWhcNMTcwODIxMDUyNzQxWjBKMQswCQYDVQQGEwJKUDEOMAwGA1UE
+CAwFVG9reW8xETAPBgNVBAoMCEZyYW5rNEREMRgwFgYDVQQDDA93d3cuZXhhbXBs
+ZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC0z9FeMynsC8+u
+dvX+LciZxnh5uRj4C9S6tNeeAlIGCfQYk0zUcNFCoCkTknNQd/YEiawDLNbxBqut
+bMDZ1aarys1a0lYmUeVLCIqvzBkPJTSQsCopQQ9V8WuT252zzNzs68dVGNdCJd5J
+NRQykpwexmnjPPv0mvj7i8XgG379TyW6P+WWV5okeUkXJ9eJS2ouDYdR2SM9BoVW
++FgxDu6BmXhozW5EfsnajFp7HL8kQClI0QOc79yuKl3492rH6bzFsFn2lfwWy9ic
+7cP8EpCTeFp1tFaD+vxBhPZkeTQ1HKx6hQ5zeHIB5ySJJZ7af2W8r4eTGYzbdRW2
+4DDHCPhZAgMBAAEwDQYJKoZIhvcNAQEFBQADgYEAQMv+BFvGdMVzkQaQ3/+2noVz
+/uAKbzpEL8xTcxYyP3lkOeh4FoxiSWqy5pGFALdPONoDuYFpLhjJSZaEwuvjI/Tr
+rGhLV1pRG9frwDFshqD2Vaj4ENBCBh6UpeBop5+285zQ4SI7q4U9oSebUDJiuOx6
++tZ9KynmrbJpTSi0+BM=
+-----END CERTIFICATE-----
diff --git a/images/test/foo-printer.sh b/images/test/foo-printer.sh