Use secure way to access parameter values in inline scripts

shipwright-io · Mar 23, 2022 · 9998ac3 · 9998ac3
1 parent 32b955c
commit 9998ac3
Show file tree

Hide file tree

Showing 8 changed files with 259 additions and 67 deletions.
diff --git a/docs/buildstrategies.md b/docs/buildstrategies.md
@@ -32,6 +32,7 @@ SPDX-License-Identifier: Apache-2.0
 - [Strategy parameters](#strategy-parameters)
 - [System parameters](#system-parameters)
 - [System parameters vs Strategy Parameters Comparison](#system-parameters-vs-strategy-parameters-comparison)
+- [Securely referencing string parameters](#securely-referencing-string-parameters)
 - [System results](#system-results)
 - [Steps Resource Definition](#steps-resource-definition)
   - [Strategies with different resources](#strategies-with-different-resources)
@@ -200,7 +201,7 @@ The build strategy provides the following parameters that you can set in a Build
 | Parameter | Description | Default |
 | -- | -- | -- |
 | `go-flags` | Value for the GOFLAGS environment variable. | Empty |
-| `go-version` | Version of Go, must match a tag from [the golang image](https://hub.docker.com/_/golang?tab=tags) | `1.16` |
+| `go-version` | Version of Go, must match a tag from [the golang image](https://hub.docker.com/_/golang?tab=tags) | `1.17` |
 | `ko-version` | Version of ko, must be either `latest` for the newest release, or a [ko release name](https://github.com/google/ko/releases) | `latest` |
 | `package-directory` | The directory inside the context directory containing the main package. | `.` |
 | `target-platform` | Target platform to be built. For example: `linux/arm64`. Multiple platforms can be provided separated by comma, for example: `linux/arm64,linux/amd64`. The value `all` will build all platforms supported by the base image. The value `current` will build the platform on which the build runs. | `current` |
@@ -412,6 +413,86 @@ Contrary to the strategy `spec.parameters`, you can use system parameters and th
 | System Parameter   |    No        |  At run-time, by the `BuildRun` controller.  |
 | Strategy Parameter |    Yes       |  At build-time, during the `BuildStrategy` creation. |
 
+## Securely referencing string parameters
+
+In build strategy steps, string parameters are referenced using `$(params.PARAM_NAME)`. This applies to system parameters, and those parameters defined in the build strategy. You can reference those parameters at many locations in the build steps, such as environment variables values, arguments, image, and more. In the Pod, all `$(params.PARAM_NAME)` tokens will be replaced by simple string replaces. This is safe in most locations but requires your attention when you define an inline script using an argument. For example:
+
+```yaml
+spec:
+  parameters:
+    - name: sample-parameter
+      description: A sample parameter
+      type: string
+  buildSteps:
+    - name: sample-step
+      command:
+        - /bin/bash
+      args:
+        - -c
+        - |
+          set -euo pipefail
+
+          some-tool --sample-argument "$(params.sample-parameter)"
+```
+
+This opens the door to script injection, for example if the user sets the `sample-parameter` to `argument-value" && malicious-command && echo "`, the resulting pod argument will look like this:
+
+```yaml
+        - |
+          set -euo pipefail
+
+          some-tool --sample-argument "argument-value" && malicious-command && echo ""
+```
+
+To securely pass a parameter value into a script-style argument, you can chose between these two approaches:
+
+1. Using environment variables. This is used in some of our sample strategies, for example [ko](../samples/buildstrategy/ko/buildstrategy_ko_cr.yaml), or [buildpacks](../samples/buildstrategy/buildpacks-v3/buildstrategy_buildpacks-v3_cr.yaml). Basically, instead of directly using the parameter inside the script, you pass it via environment variable. Using quoting, shells ensure that no command injection is possible:
+
+   ```yaml
+   spec:
+     parameters:
+       - name: sample-parameter
+         description: A sample parameter
+         type: string
+     buildSteps:
+       - name: sample-step
+         env:
+           - name: PARAM_SAMPLE_PARAMETER
+             value: $(params.sample-parameter)
+         command:
+           - /bin/bash
+         args:
+           - -c
+           - |
+             set -euo pipefail
+
+             some-tool --sample-argument "${PARAM_SAMPLE_PARAMETER}"
+   ```
+
+2. Using arguments. This is used in some of our sample build strategies, for example [buildah](../samples/buildstrategy/buildah/buildstrategy_buildah_cr.yaml). Here, you use arguments to your own inline script. Appropriate shell quoting guards against command injection.
+
+   ```yaml
+   spec:
+     parameters:
+       - name: sample-parameter
+         description: A sample parameter
+         type: string
+     buildSteps:
+       - name: sample-step
+         command:
+           - /bin/bash
+         args:
+           - -c
+           - |
+             set -euo pipefail
+
+             SAMPLE_PARAMETER="$1"
+
+             some-tool --sample-argument "${SAMPLE_PARAMETER}"
+           - --
+           - $(params.sample-parameter)
+   ```
+
 ## System results
 
 You can optionally store the size and digest of the image your build strategy created to a set of files.

diff --git a/samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml b/samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml
@@ -66,18 +66,44 @@ spec:
       # See https://github.com/moby/buildkit/blob/master/docs/rootless.md#about---oci-worker-no-process-sandbox for more information
       - name: BUILDKITD_FLAGS
         value: --oci-worker-no-process-sandbox
+      - name: PARAM_SOURCE_CONTEXT
+        value: $(params.shp-source-context)
+      - name: PARAM_DOCKERFILE
+        value: $(params.DOCKERFILE)
+      - name: PARAM_OUTPUT_IMAGE
+        value: $(params.shp-output-image)
+      - name: PARAM_INSECURE_REGISTRY
+        value: $(params.insecure-registry)
+      - name: PARAM_CACHE
+        value: $(params.cache)
       command:
         - /bin/ash
       args:
         - -c
         - |
           set -euo pipefail
 
+          # Verify the existence of the context directory
+          if [ ! -d "${PARAM_SOURCE_CONTEXT}" ]; then
+            echo -e "The context directory '${PARAM_SOURCE_CONTEXT}' does not exist."
+            echo -n "ContextDirNotFound" > '$(results.shp-error-reason.path)'
+            echo -n "The context directory '${PARAM_SOURCE_CONTEXT}' does not exist." > '$(results.shp-error-message.path)'
+            exit 1
+          fi
+
           # Prepare the file arguments
-          DOCKERFILE_PATH='$(params.shp-source-context)/$(build.dockerfile)'
+          DOCKERFILE_PATH="${PARAM_SOURCE_CONTEXT}/${PARAM_DOCKERFILE}"
           DOCKERFILE_DIR="$(dirname "${DOCKERFILE_PATH}")"
           DOCKERFILE_NAME="$(basename "${DOCKERFILE_PATH}")"
 
+          # Verify the existence of the Dockerfile
+          if [ ! -f "${DOCKERFILE_PATH}" ]; then
+            echo -e "The Dockerfile '${DOCKERFILE_PATH}' does not exist."
+            echo -n "DockerfileNotFound" > '$(results.shp-error-reason.path)'
+            echo -n "The Dockerfile '${DOCKERFILE_PATH}' does not exist." > '$(results.shp-error-message.path)'
+            exit 1
+          fi
+
           # We only have ash here and therefore no bash arrays to help add dynamic arguments (the build-args) to the build command.
 
           echo "#!/bin/ash" > /tmp/run.sh
@@ -87,18 +113,18 @@ spec:
           echo "--progress=plain \\" >> /tmp/run.sh
           echo "--frontend=dockerfile.v0 \\" >> /tmp/run.sh
           echo "--opt=filename=\"${DOCKERFILE_NAME}\" \\" >> /tmp/run.sh
-          echo "--local=context='$(params.shp-source-context)' \\" >> /tmp/run.sh
+          echo "--local=context=\"${PARAM_SOURCE_CONTEXT}\" \\" >> /tmp/run.sh
           echo "--local=dockerfile=\"${DOCKERFILE_DIR}\" \\" >> /tmp/run.sh
-          echo "--output=type=image,name='$(params.shp-output-image)',push=true,registry.insecure=$(params.insecure-registry) \\" >> /tmp/run.sh
-          if [ "$(params.cache)" == "registry" ]; then
+          echo "--output=type=image,name=\"${PARAM_OUTPUT_IMAGE}\",push=true,registry.insecure=\"${PARAM_INSECURE_REGISTRY}\" \\" >> /tmp/run.sh
+          if [ "${PARAM_CACHE}" == "registry" ]; then
             echo "--export-cache=type=inline \\" >> /tmp/run.sh
-            echo "--import-cache=type=registry,ref='$(params.shp-output-image)' \\" >> /tmp/run.sh
-          elif [ "$(params.cache)" == "disabled" ]; then
+            echo "--import-cache=type=registry,ref=\"${PARAM_OUTPUT_IMAGE}\" \\" >> /tmp/run.sh
+          elif [ "${PARAM_CACHE}" == "disabled" ]; then
             echo "--no-cache \\" >> /tmp/run.sh
           else
-            echo -e "An invalid value for the parameter 'cache' has been provided: '$(params.cache)'. Allowed values are 'disabled' and 'registry'."
+            echo -e "An invalid value for the parameter 'cache' has been provided: '${PARAM_CACHE}'. Allowed values are 'disabled' and 'registry'."
             echo -n "InvalidParameterValue" > '$(results.shp-error-reason.path)'
-            echo -n "An invalid value for the parameter 'cache' has been provided: '$(params.cache)'. Allowed values are 'disabled' and 'registry'." > '$(results.shp-error-message.path)'
+            echo -n "An invalid value for the parameter 'cache' has been provided: '${PARAM_CACHE}'. Allowed values are 'disabled' and 'registry'." > '$(results.shp-error-message.path)'
             exit 1
           fi
 

diff --git a/samples/buildstrategy/buildpacks-v3/buildstrategy_buildpacks-v3-heroku_cr.yaml b/samples/buildstrategy/buildpacks-v3/buildstrategy_buildpacks-v3-heroku_cr.yaml
@@ -37,6 +37,10 @@ spec:
       env: 
         - name: CNB_PLATFORM_API
           value: $(params.platform-api-version)
+        - name: PARAM_SOURCE_CONTEXT
+          value: $(params.shp-source-context)
+        - name: PARAM_OUTPUT_IMAGE
+          value: $(params.shp-output-image)
       command:
         - /bin/bash
       args:
@@ -82,22 +86,22 @@ spec:
           }
 
           anounce_phase "DETECTING"
-          /cnb/lifecycle/detector -app="$(params.shp-source-context)" -layers="$LAYERS_DIR"
+          /cnb/lifecycle/detector -app="${PARAM_SOURCE_CONTEXT}" -layers="$LAYERS_DIR"
 
           anounce_phase "ANALYZING"
-          /cnb/lifecycle/analyzer -layers="$LAYERS_DIR" -cache-dir="$CACHE_DIR" "$(params.shp-output-image)"
+          /cnb/lifecycle/analyzer -layers="$LAYERS_DIR" -cache-dir="$CACHE_DIR" "${PARAM_OUTPUT_IMAGE}"
 
           anounce_phase "RESTORING"
           /cnb/lifecycle/restorer -cache-dir="$CACHE_DIR"
 
           anounce_phase "BUILDING"
-          /cnb/lifecycle/builder -app="$(params.shp-source-context)" -layers="$LAYERS_DIR"
+          /cnb/lifecycle/builder -app="${PARAM_SOURCE_CONTEXT}" -layers="$LAYERS_DIR"
 
-          exporter_args=( -layers="$LAYERS_DIR" -report=/tmp/report.toml -cache-dir="$CACHE_DIR" -app="$(params.shp-source-context)")
+          exporter_args=( -layers="$LAYERS_DIR" -report=/tmp/report.toml -cache-dir="$CACHE_DIR" -app="${PARAM_SOURCE_CONTEXT}")
           grep -q "buildpack-default-process-type" "$LAYERS_DIR/config/metadata.toml" || exporter_args+=( -process-type web ) 
 
           anounce_phase "EXPORTING"
-          /cnb/lifecycle/exporter "${exporter_args[@]}" "$(params.shp-output-image)" 
+          /cnb/lifecycle/exporter "${exporter_args[@]}" "${PARAM_OUTPUT_IMAGE}"
 
           # Store the image digest
           grep digest /tmp/report.toml | tr -d ' \"\n' | sed s/digest=// > "$(results.shp-image-digest.path)"

diff --git a/samples/buildstrategy/buildpacks-v3/buildstrategy_buildpacks-v3-heroku_namespaced_cr.yaml b/samples/buildstrategy/buildpacks-v3/buildstrategy_buildpacks-v3-heroku_namespaced_cr.yaml
@@ -37,6 +37,10 @@ spec:
       env: 
         - name: CNB_PLATFORM_API
           value: $(params.platform-api-version)
+        - name: PARAM_SOURCE_CONTEXT
+          value: $(params.shp-source-context)
+        - name: PARAM_OUTPUT_IMAGE
+          value: $(params.shp-output-image)
       command:
         - /bin/bash
       args:
@@ -82,22 +86,22 @@ spec:
           }
 
           anounce_phase "DETECTING"
-          /cnb/lifecycle/detector -app="$(params.shp-source-context)" -layers="$LAYERS_DIR"
+          /cnb/lifecycle/detector -app="${PARAM_SOURCE_CONTEXT}" -layers="$LAYERS_DIR"
 
           anounce_phase "ANALYZING"
-          /cnb/lifecycle/analyzer -layers="$LAYERS_DIR" -cache-dir="$CACHE_DIR" "$(params.shp-output-image)"
+          /cnb/lifecycle/analyzer -layers="$LAYERS_DIR" -cache-dir="$CACHE_DIR" "${PARAM_OUTPUT_IMAGE}"
 
           anounce_phase "RESTORING"
           /cnb/lifecycle/restorer -cache-dir="$CACHE_DIR"
 
           anounce_phase "BUILDING"
-          /cnb/lifecycle/builder -app="$(params.shp-source-context)" -layers="$LAYERS_DIR"
+          /cnb/lifecycle/builder -app="${PARAM_SOURCE_CONTEXT}" -layers="$LAYERS_DIR"
 
-          exporter_args=( -layers="$LAYERS_DIR" -report=/tmp/report.toml -cache-dir="$CACHE_DIR" -app="$(params.shp-source-context)")
+          exporter_args=( -layers="$LAYERS_DIR" -report=/tmp/report.toml -cache-dir="$CACHE_DIR" -app="${PARAM_SOURCE_CONTEXT}")
           grep -q "buildpack-default-process-type" "$LAYERS_DIR/config/metadata.toml" || exporter_args+=( -process-type web ) 
 
           anounce_phase "EXPORTING"
-          /cnb/lifecycle/exporter "${exporter_args[@]}" "$(params.shp-output-image)" 
+          /cnb/lifecycle/exporter "${exporter_args[@]}" "${PARAM_OUTPUT_IMAGE}"
 
           # Store the image digest
           grep digest /tmp/report.toml | tr -d ' \"\n' | sed s/digest=// > "$(results.shp-image-digest.path)"

diff --git a/samples/buildstrategy/buildpacks-v3/buildstrategy_buildpacks-v3_cr.yaml b/samples/buildstrategy/buildpacks-v3/buildstrategy_buildpacks-v3_cr.yaml
@@ -39,6 +39,10 @@ spec:
       env: 
         - name: CNB_PLATFORM_API
           value: $(params.platform-api-version)
+        - name: PARAM_SOURCE_CONTEXT
+          value: $(params.shp-source-context)
+        - name: PARAM_OUTPUT_IMAGE
+          value: $(params.shp-output-image)
       command:
         - /bin/bash
       args:
@@ -84,22 +88,22 @@ spec:
           }
 
           anounce_phase "DETECTING"
-          /cnb/lifecycle/detector -app="$(params.shp-source-context)" -layers="$LAYERS_DIR"
+          /cnb/lifecycle/detector -app="${PARAM_SOURCE_CONTEXT}" -layers="$LAYERS_DIR"
 
           anounce_phase "ANALYZING"
-          /cnb/lifecycle/analyzer -layers="$LAYERS_DIR" -cache-dir="$CACHE_DIR" "$(params.shp-output-image)"
+          /cnb/lifecycle/analyzer -layers="$LAYERS_DIR" -cache-dir="$CACHE_DIR" "${PARAM_OUTPUT_IMAGE}"
 
           anounce_phase "RESTORING"
           /cnb/lifecycle/restorer -cache-dir="$CACHE_DIR"
 
           anounce_phase "BUILDING"
-          /cnb/lifecycle/builder -app="$(params.shp-source-context)" -layers="$LAYERS_DIR"
+          /cnb/lifecycle/builder -app="${PARAM_SOURCE_CONTEXT}" -layers="$LAYERS_DIR"
 
-          exporter_args=( -layers="$LAYERS_DIR" -report=/tmp/report.toml -cache-dir="$CACHE_DIR" -app="$(params.shp-source-context)")
+          exporter_args=( -layers="$LAYERS_DIR" -report=/tmp/report.toml -cache-dir="$CACHE_DIR" -app="${PARAM_SOURCE_CONTEXT}")
           grep -q "buildpack-default-process-type" "$LAYERS_DIR/config/metadata.toml" || exporter_args+=( -process-type web ) 
 
           anounce_phase "EXPORTING"
-          /cnb/lifecycle/exporter "${exporter_args[@]}" "$(params.shp-output-image)" 
+          /cnb/lifecycle/exporter "${exporter_args[@]}" "${PARAM_OUTPUT_IMAGE}"
 
           # Store the image digest
           grep digest /tmp/report.toml | tr -d ' \"\n' | sed s/digest=// > "$(results.shp-image-digest.path)"

diff --git a/samples/buildstrategy/buildpacks-v3/buildstrategy_buildpacks-v3_namespaced_cr.yaml b/samples/buildstrategy/buildpacks-v3/buildstrategy_buildpacks-v3_namespaced_cr.yaml
@@ -39,6 +39,10 @@ spec:
       env: 
         - name: CNB_PLATFORM_API
           value: $(params.platform-api-version)
+        - name: PARAM_SOURCE_CONTEXT
+          value: $(params.shp-source-context)
+        - name: PARAM_OUTPUT_IMAGE
+          value: $(params.shp-output-image)
       command:
         - /bin/bash
       args:
@@ -84,22 +88,22 @@ spec:
           }
 
           anounce_phase "DETECTING"
-          /cnb/lifecycle/detector -app="$(params.shp-source-context)" -layers="$LAYERS_DIR"
+          /cnb/lifecycle/detector -app="${PARAM_SOURCE_CONTEXT}" -layers="$LAYERS_DIR"
 
           anounce_phase "ANALYZING"
-          /cnb/lifecycle/analyzer -layers="$LAYERS_DIR" -cache-dir="$CACHE_DIR" "$(params.shp-output-image)"
+          /cnb/lifecycle/analyzer -layers="$LAYERS_DIR" -cache-dir="$CACHE_DIR" "${PARAM_OUTPUT_IMAGE}"
 
           anounce_phase "RESTORING"
           /cnb/lifecycle/restorer -cache-dir="$CACHE_DIR"
 
           anounce_phase "BUILDING"
-          /cnb/lifecycle/builder -app="$(params.shp-source-context)" -layers="$LAYERS_DIR"
+          /cnb/lifecycle/builder -app="${PARAM_SOURCE_CONTEXT}" -layers="$LAYERS_DIR"
 
-          exporter_args=( -layers="$LAYERS_DIR" -report=/tmp/report.toml -cache-dir="$CACHE_DIR" -app="$(params.shp-source-context)")
+          exporter_args=( -layers="$LAYERS_DIR" -report=/tmp/report.toml -cache-dir="$CACHE_DIR" -app="${PARAM_SOURCE_CONTEXT}")
           grep -q "buildpack-default-process-type" "$LAYERS_DIR/config/metadata.toml" || exporter_args+=( -process-type web ) 
 
           anounce_phase "EXPORTING"
-          /cnb/lifecycle/exporter "${exporter_args[@]}" "$(params.shp-output-image)" 
+          /cnb/lifecycle/exporter "${exporter_args[@]}" "${PARAM_OUTPUT_IMAGE}"
 
           # Store the image digest
           grep digest /tmp/report.toml | tr -d ' \"\n' | sed s/digest=// > "$(results.shp-image-digest.path)"

diff --git a/samples/buildstrategy/ko/buildstrategy_ko_cr.yaml b/samples/buildstrategy/ko/buildstrategy_ko_cr.yaml
@@ -56,6 +56,16 @@ spec:
           value: /tekton/home
         - name: GOFLAGS
           value: $(params.go-flags)
+        - name: PARAM_OUTPUT_IMAGE
+          value: $(params.shp-output-image)
+        - name: PARAM_SOURCE_CONTEXT
+          value: $(params.shp-source-context)
+        - name: PARAM_TARGET_PLATFORM
+          value: $(params.target-platform)
+        - name: PARAM_PACKAGE_DIRECTORY
+          value: $(params.package-directory)
+        - name: PARAM_KO_VERSION
+          value: $(params.ko-version)
       command:
         - /bin/bash
       args:
@@ -69,12 +79,10 @@ spec:
           # - a URL with a tag but without a port: registry/image:tag
           # - a URL with both a tag and a port: registry:port/image:tag
 
-          IMAGE=$(params.shp-output-image)
-
           REPO=
           TAG=
 
-          IFS=':' read -ra PARTS <<< "${IMAGE}"
+          IFS=':' read -ra PARTS <<< "${PARAM_OUTPUT_IMAGE}"
           for PART in "${PARTS[@]}"; do
             if [ "${REPO}" == "" ]; then
               REPO="${PART}"
@@ -89,7 +97,7 @@ spec:
           done
 
           # Determine the ko version
-          KO_VERSION='$(params.ko-version)'
+          KO_VERSION="${PARAM_KO_VERSION}"
           if [ "${KO_VERSION}" == "latest" ]; then
             KO_VERSION=$(curl --silent "https://api.github.com/repos/google/ko/releases/latest" | grep '"tag_name":' | sed -E 's/.*"([^"]+)".*/\1/')
           fi
@@ -107,7 +115,7 @@ spec:
           curl -f -s -L "https://github.com/google/ko/releases/download/${KO_VERSION_WITH_V}/ko_${KO_VERSION_WITHOUT_V}_$(uname)_$(uname -m | sed 's/aarch64/arm64/').tar.gz" | tar xzf - -C /tmp ko
 
           # Determine the platform
-          PLATFORM='$(params.target-platform)'
+          PLATFORM="${PARAM_TARGET_PLATFORM}"
           if [ "${PLATFORM}" == "current" ]; then
             PLATFORM="$(uname | tr '[:upper:]' '[:lower:]')/$(uname -m | sed -e 's/x86_64/amd64/' -e 's/aarch64/arm64/')"
           fi
@@ -118,14 +126,14 @@ spec:
 
           # Run ko
 
-          export GOROOT=$(go env GOROOT)
+          export GOROOT="$(go env GOROOT)"
           export KO_DOCKER_REPO="${REPO}"
 
-          pushd '$(params.shp-source-context)' > /dev/null
+          pushd "${PARAM_SOURCE_CONTEXT}" > /dev/null
             if [ "${TAG}" == "" ]; then
-              /tmp/ko publish '$(params.package-directory)' --bare --oci-layout-path=/tmp/layout --platform="${PLATFORM}"
+              /tmp/ko publish "${PARAM_PACKAGE_DIRECTORY}" --bare --oci-layout-path=/tmp/layout --platform="${PLATFORM}"
             else
-              /tmp/ko publish '$(params.package-directory)' --bare --oci-layout-path=/tmp/layout --platform="${PLATFORM}" --tags="${TAG}"
+              /tmp/ko publish "${PARAM_PACKAGE_DIRECTORY}" --bare --oci-layout-path=/tmp/layout --platform="${PLATFORM}" --tags="${TAG}"
             fi
           popd > /dev/null