fix: proper validation of user variables (#284)

* refactor: move var initalization to top * fix: extract USER_HOME and use new test setup * revert root function * refactor: move constants to util.sh * fix: evaluate root id later * fix: make env file posix compatible * chore: quote whole path Co-authored-by: Michael Kriese <[email protected]>
containerbase · Feb 6, 2022 · fddc530 · fddc530
1 parent 0c06e90
commit fddc530
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 48 deletions.
diff --git a/src/usr/local/bin/install-buildpack b/src/usr/local/bin/install-buildpack
@@ -2,33 +2,35 @@
 
 set -e
 
-# shellcheck source=/dev/null
-. /usr/local/buildpack/util.sh
-
-# no duplicate installs
-if [[ -n "${BUILDPACK+x}" ]]; then
-  echo "BUILDPACK defined - skipping: ${BUILDPACK}"
-  exit 1;
-fi
-
-require_distro
-require_root
-
-if [[ -z "${USER_NAME+x}" ]]; then
+# USER_NAME and USER_ID need to be defined as the
+# decision which paths are used are based on this information
+if [[ -z "${USER_NAME}" ]]; then
   export USER_NAME=user
   echo "No USER_NAME defined - using: ${USER_NAME}"
 fi
 
-if [[ -z "${USER_ID+x}" ]]; then
+if [[ -z "${USER_ID}" ]]; then
   export USER_ID=1000
   echo "No USER_ID defined - using: ${USER_ID}"
 fi
 
+# shellcheck source=/dev/null
+. /usr/local/buildpack/util.sh
+
 if [[ "${BASH_ENV}" != "${ENV_FILE}" ]]; then
   echo "Wrong BASH_ENV defined - skipping: ${BASH_ENV}"
   exit 1;
 fi
 
+# no duplicate installs
+if [[ -n "${BUILDPACK+x}" ]]; then
+  echo "BUILDPACK defined - skipping: ${BUILDPACK}"
+  exit 1;
+fi
+
+require_distro
+require_root
+
 setup_env_files
 
 echo "APT::Install-Recommends \"false\";" | tee -a /etc/apt/apt.conf.d/buildpack.conf

diff --git a/src/usr/local/buildpack/util.sh b/src/usr/local/buildpack/util.sh
@@ -4,6 +4,12 @@
 DIR="${BASH_SOURCE%/*}"
 if [[ ! -d "$DIR" ]]; then DIR="$PWD"; fi
 
+# CONSTANTS
+export ENV_FILE=/usr/local/etc/env
+export ROOT_DIR=/usr/local
+# shellcheck disable=SC2153
+export USER_HOME="/home/${USER_NAME}"
+
 # source the helper files
 # shellcheck source=/dev/null
 . "${DIR}/utils/environment.sh"

diff --git a/src/usr/local/buildpack/utils/environment.sh b/src/usr/local/buildpack/utils/environment.sh
@@ -1,7 +1,5 @@
 #!/bin/bash
 
-export ENV_FILE=/usr/local/etc/env
-
 function refreshenv () {
   if [[ -r "$ENV_FILE" ]]; then
     # shellcheck source=/dev/null
@@ -45,7 +43,7 @@ function find_tool_env () {
 function export_tool_env () {
   local install_dir
   install_dir=$(get_install_dir)
-  if [[ -z "${TOOL_NAME+x}" ]]; then
+  if [[ -z "${TOOL_NAME}" ]]; then
     echo "No TOOL_NAME defined - skipping: ${TOOL_NAME}" >&2
     exit 1;
   fi
@@ -72,24 +70,27 @@ function get_tool_version_env () {
 
 function setup_env_files () {
   # env helper, loads tool specific env
+  local install_dir
+  install_dir=$(get_install_dir)
+
   cat >> "$ENV_FILE" <<- EOM
-export BUILDPACK=1 USER_NAME="${USER_NAME}" USER_ID="${USER_ID}" USER_HOME="/home/${USER_NAME}"
+export BUILDPACK=1 USER_NAME="${USER_NAME}" USER_ID="${USER_ID}" USER_HOME="${USER_HOME}"
 
 # openshift override unknown user home
 if [ "\${EUID}" != 0 ]; then
   export HOME="\${USER_HOME}"
 fi
 
-if [ -d /usr/local/env.d ]; then
-  for i in /usr/local/env.d/*.sh; do
+if [ -d "${install_dir}/env.d" ]; then
+  for i in "${install_dir}/env.d"/*.sh; do
     if [ -r \$i ]; then
       . \$i
     fi
   done
   unset i
 fi
-if [ -d /home/"${USER_NAME}"/env.d ]; then
-  for i in /home/"${USER_NAME}"/env.d/*.sh; do
+if [ -d "${USER_HOME}/env.d" ]; then
+  for i in "${USER_HOME}/env.d"/*.sh; do
     if [ -r \$i ]; then
       . \$i
     fi

diff --git a/src/usr/local/buildpack/utils/filesystem.sh b/src/usr/local/buildpack/utils/filesystem.sh
@@ -1,8 +1,5 @@
 #!/bin/bash
 
-# Constants
-ROOT_DIR=/usr/local
-
 function get_install_dir () {
   if [ "$(is_root)" -eq 0 ]; then
     echo "${ROOT_DIR}"

diff --git a/test/bash/environment.bats b/test/bash/environment.bats
@@ -2,18 +2,21 @@ setup() {
   load '../../node_modules/bats-support/load'
   load '../../node_modules/bats-assert/load'
 
-  TEST_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")" >/dev/null 2>&1 && pwd)"
+    TEST_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")" >/dev/null 2>&1 && pwd)"
+    TEST_ROOT_DIR=$(mktemp -u)
 
-  # Not used yet, will be used after the refactoring
-  TEST_ROOT_DIR=$(mktemp -u)
+    load "$TEST_DIR/../../src/usr/local/buildpack/util.sh"
 
-  # Not used yet
-  USER_NAME=user
-  USER_ID=1000
-  # Not needed in the future
-  USER_HOME=${TEST_ROOT_DIR}
+    # load test overwrites
+    load "$TEST_DIR/util.sh"
 
-  load "$TEST_DIR/../../src/usr/local/buildpack/util.sh"
+    # set directories for test
+    ROOT_DIR="${TEST_ROOT_DIR}/root"
+    USER_HOME="${TEST_ROOT_DIR}/user"
+    ENV_FILE="${TEST_ROOT_DIR}/env"
+
+    # set default test user
+    TEST_ROOT_USER=1000
 }
 
 teardown() {
@@ -25,12 +28,10 @@ teardown() {
   local TOOL_NAME=foo
   local TOOL_VERSION=1.2.3
 
-  mkdir -p "${TEST_ROOT_DIR}/env.d"
+  mkdir -p "${TEST_ROOT_DIR}/user/env.d"
 
-  # TODO(Chumper): This should fail
   TOOL_NAME= run export_tool_env
-  # assert_failure
-  assert_success
+  assert_failure
 
   export_tool_env FOO_HOME 123
   assert [ "${FOO_HOME}" = "123" ]
@@ -50,12 +51,10 @@ teardown() {
   local TOOL_NAME=foo
   local TOOL_VERSION=1.2.3
 
-  mkdir -p "${TEST_ROOT_DIR}/env.d"
+  mkdir -p "${TEST_ROOT_DIR}/user/env.d"
 
-  # TODO(Chumper): This should fail
   TOOL_NAME= run export_tool_env
-  # assert_failure
-  assert_success
+  assert_failure
 
   export_tool_env FOO_HOME 123
   assert [ "${FOO_HOME}" = "123" ]
@@ -95,7 +94,7 @@ teardown() {
   local TOOL_NAME=foo
   local TOOL_VERSION=1.2.3
 
-  mkdir -p "${TEST_ROOT_DIR}/env.d"
+  mkdir -p "${TEST_ROOT_DIR}/user/env.d"
 
   local old_path=$PATH
 

diff --git a/test/bash/linking.bats b/test/bash/linking.bats
@@ -3,13 +3,21 @@ setup() {
   load '../../node_modules/bats-support/load'
   load '../../node_modules/bats-assert/load'
 
-  TEST_DIR="$( cd "$( dirname "$BATS_TEST_FILENAME" )" >/dev/null 2>&1 && pwd )"
+    TEST_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")" >/dev/null 2>&1 && pwd)"
+    TEST_ROOT_DIR=$(mktemp -u)
 
-  # Not used yet, but will be later
-  TEST_ROOT_DIR=$(mktemp -u)
-  USER_HOME=$TEST_ROOT_DIR
+    load "$TEST_DIR/../../src/usr/local/buildpack/util.sh"
 
-  load "$TEST_DIR/../../src/usr/local/buildpack/util.sh"
+    # load test overwrites
+    load "$TEST_DIR/util.sh"
+
+    # set directories for test
+    ROOT_DIR="${TEST_ROOT_DIR}/root"
+    USER_HOME="${TEST_ROOT_DIR}/user"
+    ENV_FILE="${TEST_ROOT_DIR}/env"
+
+    # set default test user
+    TEST_ROOT_USER=1000
 }
 
 teardown() {