diff --git a/README.md b/README.md index a4ffaa16..1a206b1d 100644 --- a/README.md +++ b/README.md @@ -211,15 +211,19 @@ Additionally, volumes may be specified via the agent environment variable `BUILD When set to true, it will activate interpolation of variables in the elements within the `push` configuration array. When turned off (the default), attempting to use variables will fail as the literal `$VARIABLE_NAME` string will be used as the target.. -:warning: **Important:** this is considered an unsafe option as the most compatible way to achieve this is to run the strings through `eval` which could lead to arbitrary code execution or information leaking if you don't have complete control of the pipeline +:warning: **Important:** this is considered an unsafe option as the most compatible way to achieve this is to run the strings through `eval` which could lead to arbitrary code execution or information leaking if you don't have complete control of the pipeline. On a system with `envsubst` available, that tool will be used in preference to `eval`. See `expand-vars-allowlist` to restrict the variables allowed to be substituted when using `envsubst`. Note that rules regarding [environment variable interpolation](https://buildkite.com/docs/pipelines/environment-variables#runtime-variable-interpolation) apply here. That means that `$VARIABLE_NAME` is resolved at pipeline upload time, whereas `$$VARIABLE_NAME` will be at run time. All things being equal, you likely want to use `$$VARIABLE_NAME` on the variables mentioned in this option. +#### `expand-vars-allowlist` (push or run, string, when using one of the `expand-*-vars` options, and on a system with `envsubst`) + +This variable is used to define the commands `SHELL-FORMAT` argument to `envsubst`, which defines the environment variable allowlist so that unexpected interpolations are not permitted. It is only relevant when using `expand-push-vars` or `expand-volume-vars` + #### `expand-volume-vars` (run only, boolean, unsafe) When set to true, it will activate interpolation of variables in the elements of the `volumes` configuration array. When turned off (the default), attempting to use variables will fail as the literal `$VARIABLE_NAME` string will be passed to the `-v` option. -:warning: **Important:** this is considered an unsafe option as the most compatible way to achieve this is to run the strings through `eval` which could lead to arbitrary code execution or information leaking if you don't have complete control of the pipeline +:warning: **Important:** this is considered an unsafe option as the most compatible way to achieve this is to run the strings through `eval` which could lead to arbitrary code execution or information leaking if you don't have complete control of the pipeline. On a system with `envsubst` available, that tool will be used in preference to `eval`. See `expand-vars-allowlist` to restrict the variables allowed to be substituted when using `envsubst`. Note that rules regarding [environment variable interpolation](https://buildkite.com/docs/pipelines/environment-variables#runtime-variable-interpolation) apply here. That means that `$VARIABLE_NAME` is resolved at pipeline upload time, whereas `$$VARIABLE_NAME` will be at run time. All things being equal, you likely want to use `$$VARIABLE_NAME` on the variables mentioned in this option. diff --git a/commands/push.sh b/commands/push.sh index 71558775..3cfef25f 100755 --- a/commands/push.sh +++ b/commands/push.sh @@ -41,7 +41,7 @@ prebuilt_image_namespace="$(plugin_read_config PREBUILT_IMAGE_NAMESPACE 'docker- # Then we figure out what to push, and where for line in $(plugin_read_list PUSH) ; do if [[ "$(plugin_read_config EXPAND_PUSH_VARS 'false')" == "true" ]]; then - push_target=$(eval echo "$line") + push_target="$(expand_var "$line")" else push_target="$line" fi diff --git a/lib/run.bash b/lib/run.bash index f9a263eb..ae8e9dc8 100644 --- a/lib/run.bash +++ b/lib/run.bash @@ -92,7 +92,7 @@ expand_relative_volume_path() { local path if [[ "$(plugin_read_config EXPAND_VOLUME_VARS 'false')" == "true" ]]; then - path=$(eval echo "$1") + path="$(expand_var "$1")" else path="$1" fi diff --git a/lib/shared.bash b/lib/shared.bash index 303be82e..503e5d70 100644 --- a/lib/shared.bash +++ b/lib/shared.bash @@ -322,6 +322,31 @@ function retry { done } +# Expands the env vars in a string, using envsubst if present, falling back to +# eval when it's missing. +function expand_var() { + # Try to use the safest approach possible + if command -v envsubst > /dev/null; then + expand_var_with_envsubst "$@" + else + expand_var_with_eval "$1" + fi +} + +function expand_var_with_envsubst() { + allowlist="$(plugin_read_config 'EXPAND_VARS_ALLOWLIST' '')" + if [[ "$allowlist" == "" ]]; then + envsubst <<<"$1" + else + envsubst "$allowlist" <<<"$1" + fi +} + +function expand_var_with_eval() { + echo "$(eval echo "$1")" +} + + function is_windows() { [[ "$OSTYPE" =~ ^(win|msys|cygwin) ]] } diff --git a/plugin.yml b/plugin.yml index 51251335..2936a711 100755 --- a/plugin.yml +++ b/plugin.yml @@ -96,6 +96,8 @@ configuration: type: string expand-push-vars: type: boolean + expand-vars-allowlist: + type: string expand-volume-vars: type: boolean graceful-shutdown: @@ -203,6 +205,7 @@ configuration: env-propagation-list: [ run ] environment: [ run ] expand-push-vars: [ push ] + expand-vars-allowlist: [ push, run ] expand-volume-vars: [ volumes ] graceful-shutdown: [ run ] leave-volumes: [ run ] diff --git a/tests/shared_lib.bats b/tests/shared_lib.bats new file mode 100644 index 00000000..7a93a882 --- /dev/null +++ b/tests/shared_lib.bats @@ -0,0 +1,89 @@ +#!/usr/bin/env bats + +load "${BATS_PLUGIN_PATH}/load.bash" +load '../lib/shared.bash' + +@test "expand_var works" { + export MY_VAR="llamas" + MY_STRING="foo:bar:\$MY_VAR" + + run expand_var "$MY_STRING" + + assert_success + assert_output "foo:bar:llamas" +} + +@test "expand_var works via envsubst" { + export MY_VAR="llamas" + MY_STRING="foo:bar:\$MY_VAR" + + ENVSUB_STUB_STDIN="$BATS_TEST_TMPDIR/envsubst_input" + stub envsubst "cat > '$ENVSUB_STUB_STDIN'; echo 'foo:bar:llamas'" + + run expand_var_with_envsubst "$MY_STRING" + + assert_success + assert_output "foo:bar:llamas" + + run cat "$ENVSUB_STUB_STDIN" + + assert_success + assert_output "$MY_STRING" + + unstub envsubst +} + +@test "expand_var works via envsubst with an allowlist" { + export MY_VAR="llamas" + MY_STRING="foo:bar:\$MY_VAR" + ALLOWLIST='$MY_VAR' + export BUILDKITE_PLUGIN_DOCKER_COMPOSE_EXPAND_VARS_ALLOWLIST="$ALLOWLIST" + + ENVSUB_STUB_STDIN="$BATS_TEST_TMPDIR/envsubst_input" + stub envsubst "'$ALLOWLIST' : cat > '$ENVSUB_STUB_STDIN'; echo 'foo:bar:llamas'" + + run expand_var_with_envsubst "$MY_STRING" + + assert_success + assert_output "foo:bar:llamas" + + run cat "$ENVSUB_STUB_STDIN" + + assert_success + assert_output "$MY_STRING" + + unstub envsubst +} + +@test "expand_var works via envsubst with an allowlist not including the var" { + export MY_VAR="llamas" + export MY_OTHER_VAR="more_llamas" + MY_STRING="foo:bar:\$MY_VAR:\$MY_OTHER_VAR" + ALLOWLIST='$MY_OTHER_VAR' + export BUILDKITE_PLUGIN_DOCKER_COMPOSE_EXPAND_VARS_ALLOWLIST="$ALLOWLIST" + + ENVSUB_STUB_STDIN="$BATS_TEST_TMPDIR/envsubst_input" + stub envsubst "'$ALLOWLIST' : cat > '$ENVSUB_STUB_STDIN'; echo 'foo:bar:\$MY_VAR:more_llamas'" + + run expand_var_with_envsubst "$MY_STRING" + + assert_success + assert_output "foo:bar:\$MY_VAR:more_llamas" + + run cat "$ENVSUB_STUB_STDIN" + + assert_success + assert_output "$MY_STRING" + + unstub envsubst +} + +@test "expand_var works via eval" { + export MY_VAR="llamas" + MY_STRING="foo:bar:\$MY_VAR" + + run expand_var_with_eval "$MY_STRING" + + assert_success + assert_output "foo:bar:llamas" +}