From a07180399b884eaacdea31eface65e97808181a8 Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Thu, 29 Oct 2026 01:54:48 +0200 Subject: [PATCH 1/6] fix(CF-2507): mark the shell-evaluated arguments of codefresh-run and helm The engine substitutes Codefresh variables into step arguments as text, and these steps evaluate some arguments with a shell, so a branch name or a commit message could run commands. The arguments schemas now mark these arguments with "x-shell-evaluated" ("unquoted" or "double-quoted"). In the marked arguments the engine passes the Git variables to the shell as environment variables. Engines without this support ignore the keyword, so the steps work as before. --- graduated/codefresh-run/step.yaml | 21 ++++++++++++++++++++- graduated/helm/step.yaml | 23 ++++++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/graduated/codefresh-run/step.yaml b/graduated/codefresh-run/step.yaml index 917885e97..ad3aaa899 100644 --- a/graduated/codefresh-run/step.yaml +++ b/graduated/codefresh-run/step.yaml @@ -3,7 +3,7 @@ kind: step-type metadata: name: codefresh-run title: Run a Codefresh pipeline - version: 1.6.0 + version: 1.6.1 isPublic: true description: Run a Codefresh pipeline by ID or name and attach the created build logs. sources: @@ -40,6 +40,11 @@ metadata: latest: true id: 5e0231f76e1ebe58d5d37a37 spec: + # "x-shell-evaluated" marks the arguments that stepsTemplate puts into the shell command, and where: + # "unquoted", or "double-quoted" when the template puts the argument inside double quotes. + # In these arguments the engine passes the Git variables, such as CF_BRANCH, to the shell as environment variables, + # so that the shell doesn't run commands from a branch name or a commit message. + # Update the marks when you change how stepsTemplate uses an argument (see ShellArguments.js in the engine). arguments: |- { "definitions": {}, @@ -58,21 +63,25 @@ spec: "default": "0.89.1" }, "PIPELINE_ID": { + "x-shell-evaluated": "double-quoted", "type": "string", "description": "Name or ID of pipeline to run. In the UI, select Pipelines. Click pipeline name, then select Settings, and get the Name and ID." }, "TRIGGER_ID": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Name or ID of trigger for specified pipeline. Mandatory when BRANCH or SHA are defined. In the UI, select Pipelines. Click pipeline name, then select Workflows > Triggers. Click Edit and get the Name. ", "default": "" }, "BRANCH": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Branch for which to run the pipeline. When defined, TRIGGER_ID must also be defined.", "examples": ["master"], "default": "" }, "SHA": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Git commit hash. When defined, TRIGGER_ID must also be defined.", "examples": ["c777d850ff67969cb4eb9e041d7edeb367705e12"], @@ -99,12 +108,14 @@ spec: "default": false }, "RUNTIME_NAME": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Runtime environment to run the pipeline.", "examples": ["my-runtime-name"], "default": "" }, "PACK_NAME": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Resources size (small, medium or large).", "default": "" @@ -120,6 +131,7 @@ spec: "default": true }, "CONTEXT": { + "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -129,6 +141,7 @@ spec: "default": [] }, "VARIABLE": { + "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -138,6 +151,7 @@ spec: "default": [] }, "SECRETS": { + "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -147,12 +161,14 @@ spec: "default": [] }, "VARIABLE_FILE": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Import build variables from a file.", "examples": ["/codefresh/volume/my-repo/var_file_example.yml"], "default": "" }, "TIMEOUT_MINS": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Timeout in minutes after which to consider pipeline build as failed.", "examples": ["33.3", "45"], @@ -164,6 +180,7 @@ spec: "default": false }, "ONLY": { + "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -173,6 +190,7 @@ spec: "default": [] }, "SKIP": { + "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -182,6 +200,7 @@ spec: "default": [] }, "YAML": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Path to pipeline's yaml to be used as override for this execution.", "default": "" diff --git a/graduated/helm/step.yaml b/graduated/helm/step.yaml index ffe04b7ba..39ab12d05 100644 --- a/graduated/helm/step.yaml +++ b/graduated/helm/step.yaml @@ -2,7 +2,7 @@ kind: step-type version: '1.0' metadata: name: helm - version: 1.1.12 + version: 1.1.13 title: Release a Helm chart isPublic: true description: Update or install a Helm chart. @@ -73,6 +73,12 @@ metadata: commands: - helm list spec: + # "x-shell-evaluated" marks the arguments that end up in a shell command, and where: "unquoted", or "double-quoted" + # when the command has the argument inside double quotes. The entrypoint script of the image builds the command + # from the environment variables that stepsTemplate creates from the arguments. + # In these arguments the engine passes the Git variables, such as CF_BRANCH, to the shell as environment variables, + # so that the shell doesn't run commands from a branch name or a commit message. + # Update the marks when you change how an argument is used (see ShellArguments.js in the engine). arguments: |- { "definitions": {}, @@ -84,6 +90,7 @@ spec: ], "properties": { "kube_context": { + "x-shell-evaluated": "double-quoted", "type": "string", "description": "Kubernetes context to use (the name of the cluster as configured in Codefresh) (required)" }, @@ -100,10 +107,12 @@ spec: "description": "Add helm repositories from contexts for auth action" }, "chart_name": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Helm chart name to release (path to chart folder, or name of packaged chart)" }, "release_name": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Helm release name" }, @@ -112,19 +121,23 @@ spec: "description": "The helm operation mode is set by the ACTION variable, where the value is install/auth/push" }, "namespace": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Target Kubernetes namespace" }, "tiller_namespace": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Kubernetes namespace where tiller is at" }, "chart_version": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Application chart version to install" }, "app_version": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Version of the application. This field is informational, and has no impact on chart version calculations." }, @@ -147,10 +160,12 @@ spec: "description": "Version of cfstep-helm image" }, "chart_repo_url": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Helm chart repository URL (overriden by injected Helm repository context)" }, "custom_value_files": { + "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -158,6 +173,7 @@ spec: "description": "Values file to provide to Helm (as --values or -f). see usage information below" }, "custom_values": { + "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -165,10 +181,12 @@ spec: "description": "Variables to provide to Helm (as --set). see usage information below" }, "cmd_ps": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Variable to provide other Helm cli flags. For example: '--wait --timeout', etc" }, "set_file": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "Set values from respective files specified via the command line (can specify multiple or separate values with commas: key1=path1,key2=path2)" }, @@ -177,10 +195,12 @@ spec: "description": "Waits until all Pods are in a ready state, PVCs are bound, Deployments have minimum Pods in ready state and Services have an IP address before marking the release as successful" }, "timeout": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "A value in seconds to wait for Kubernetes commands to complete" }, "commands": { + "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -196,6 +216,7 @@ spec: "description": "Add credentials to helm command as arguments (username, password). Otherwise its will be added to url http(s)://username:password@url. Should be enabled for JFrog Artifactory helm repository." }, "chart_subdir": { + "x-shell-evaluated": "unquoted", "type": "string", "description": "optional subfolder (inside JFrog Artifactory helm repository)" }, From 49f4542971d34e18eaf76b38c64244be2e0da0d1 Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Thu, 29 Oct 2026 02:34:59 +0200 Subject: [PATCH 2/6] trigger commit --- tests/cf2507-breaking-change-notice.md | 116 +++++ tests/cf2507-interpolation-security.yml | 566 ++++++++++++++++++++++++ tests/cf2507-pr-descriptions.md | 199 +++++++++ tests/cf2507-security-advisory.md | 133 ++++++ tests/cf2507-support-runbook.md | 134 ++++++ 5 files changed, 1148 insertions(+) create mode 100644 tests/cf2507-breaking-change-notice.md create mode 100644 tests/cf2507-interpolation-security.yml create mode 100644 tests/cf2507-pr-descriptions.md create mode 100644 tests/cf2507-security-advisory.md create mode 100644 tests/cf2507-support-runbook.md diff --git a/tests/cf2507-breaking-change-notice.md b/tests/cf2507-breaking-change-notice.md new file mode 100644 index 000000000..961fc9c8d --- /dev/null +++ b/tests/cf2507-breaking-change-notice.md @@ -0,0 +1,116 @@ +# Breaking change notice: argument handling in the codefresh-run and helm steps + + + +**Subject:** Action may be needed: the codefresh-run and helm pipeline steps change how they handle argument values on [TBD] + +Hi, + +On **[TBD release date]** we're releasing new versions of two official pipeline steps, **Run a Codefresh pipeline** (`codefresh-run` 1.7.0) and **Release a Helm chart** (`helm` 1.1.13), together with rebuilt `cfstep-helm` images. This is a security hardening change. + +Most pipelines keep working without any changes. This notice helps you check whether yours need an update, and explains how to update them. + +## What's changing + +Until now, these steps passed their argument values through a shell before using them. The shell evaluated some characters in the values. After the update, the steps use argument values exactly as you write them. + +| You write | Before the update | After the update | +|---|---|---| +| `${{MY_VAR}}` | The value of `MY_VAR` | No change: the value of `MY_VAR` | +| `$MY_VAR` or `${MY_VAR}` | The value of `MY_VAR` | The text `$MY_VAR` | +| `$(command)` or `` `command` `` | The output of the command | The text `$(command)` | +| `values-*.yaml` | The matching file names | The text `values-*.yaml` | +| `~/file` | The path in the home directory | The text `~/file` | +| `"main --no-cache"` in one argument | Several flags | One value: `main --no-cache` | + +Quotes and escaped commas in Helm values keep working the same way. For example, `profiles_arr="{one,two,three}"` and `profiles_str="one\,two\,three"` are passed to Helm as before. + +## Who's affected + +Your pipeline needs an update if a `codefresh-run` or `helm` step has an argument that uses `$VAR`, `$(...)`, backticks, `*`, or `~`. For the `helm` step, this includes `custom_values`, `custom_value_files`, `cmd_ps`, `kube_context`, `namespace`, and `release_name`. + +These pipelines get the new behavior: + +- Pipelines that use `type: codefresh-run` or `type: helm` without a version, from [TBD release date]. +- All `helm` step versions, because the rebuilt `cfstep-helm` images are published with the same tags. +- Pipelines that run a `cfstep-helm` image directly in a freestyle step. + +## How to check your pipelines + +1. Export the pipelines you store in Codefresh: + + ```bash + codefresh get pipelines --limit 1000 -o yaml > pipelines.yaml + ``` + +2. For pipelines stored in Git, check their `codefresh.yml` files in your repositories. +3. In each `codefresh-run` and `helm` step, look at the `arguments` for `$` (not followed by `{{`), `` ` ``, `*`, and `~`. For example: + + ```bash + grep -nE '\$[A-Za-z_(]|\$\{[^{]|`' pipelines.yaml + ``` + + This also finds `$VAR` in the `commands` of freestyle steps. Those don't change, so you can ignore them. + +You can also run a pipeline with the new step versions and look for a warning in the step log (see [Temporary option](#temporary-option) below). + +## How to update your pipelines + +| Before | After | +|---|---| +| `custom_values: [image_tag=$MY_TAG]` | `custom_values: [image_tag=${{MY_TAG}}]` | +| `VARIABLE: [IMAGE=$IMAGE_NAME]` | `VARIABLE: [IMAGE=${{IMAGE_NAME}}]` | +| `cmd_ps: --set version=$(cat VERSION)` | Read the file in a previous step: `cf_export VERSION=$(cat VERSION)`, then use `cmd_ps: --set version=${{VERSION}}` | +| `cmd_ps: --values values-*.yaml` | List the files in `custom_value_files` | +| `BRANCH: main --no-cache` | `BRANCH: main` and `NO_CACHE: true` | + +Variables you export with `cf_export` are available as `${{VAR}}` in all later steps. + +## Temporary option + +If you need more time to update a pipeline, you can get the previous behavior back for that step: + +| Step | Setting | +|---|---| +| `codefresh-run` 1.7.0 or later | `LEGACY_SHELL_EXPANSION: true` | +| `helm` 1.1.13 or later | `legacy_shell_expansion: true` | +| `cfstep-helm` image used directly | environment variable `LEGACY_SHELL_EXPANSION=true` | + +For example: + +```yaml +deploy: + type: helm:1.1.13 + arguments: + chart_name: my-chart + release_name: my-release + kube_context: my-cluster + custom_values: + - image_tag=$MY_TAG + legacy_shell_expansion: true # Temporary: remove after replacing $MY_TAG with ${{MY_TAG}} +``` + +When the option is enabled, the step log shows a warning with the names of the arguments that still need an update. If the warning says that no arguments use shell syntax, you can remove the option right away. + +This option is for migration only. It's less secure, and we'll remove it on **[TBD removal date]**. Please update your pipelines before then. + +Older `helm` step versions don't accept the new argument. To use it, upgrade the step to `helm` 1.1.13. + +## Timeline + +| Date | What happens | +|---|---| +| [TBD] | This notice | +| [TBD] | New step versions and rebuilt `cfstep-helm` images released | +| [TBD] | The temporary option is removed | + +## Questions + +If you have questions or need help updating your pipelines, contact [Codefresh support](https://support.codefresh.io). + +Happy deployments! \ No newline at end of file diff --git a/tests/cf2507-interpolation-security.yml b/tests/cf2507-interpolation-security.yml new file mode 100644 index 000000000..fe0d2a51c --- /dev/null +++ b/tests/cf2507-interpolation-security.yml @@ -0,0 +1,566 @@ +# CF-2507: Remote Code Execution via Codefresh Steps - verification pipeline +# +# Verifies the fixes in: +# - steps: graduated/codefresh-run (1.7.0), graduated/helm (1.1.13) +# - cfstep-helm: CF-2507-fix-interpolation-security (alpine) / CF-2507-fix-interpolation-security-debian (debian) images +# +# Every malicious payload tries to create its own marker file in /codefresh/volume/cf2507-markers. +# The shared volume is visible to all steps of the build, so a marker means that the payload was executed. +# Benign values check that existing pipelines keep working (quotes, spaces, colons, helm list syntax). +# +# The pipeline calls itself through codefresh-run: the child build (CF2507_MODE=child) verifies that +# the variables arrived unchanged. Do not define CF2507_MODE as a pipeline variable. +# +# Setup +# 1. Build the cfstep-helm images from both CF-2507 branches and note the image tags. +# 2. Publish private copies of the step types from this branch (replace ACCOUNT with your account name): +# yq '.metadata.name = "ACCOUNT/codefresh-run-cf2507" | .metadata.isPublic = false' graduated/codefresh-run/step.yaml > /tmp/codefresh-run-cf2507.yaml +# yq '.metadata.name = "ACCOUNT/helm-cf2507" | .metadata.isPublic = false' graduated/helm/step.yaml > /tmp/helm-cf2507.yaml +# codefresh create step-type -f /tmp/codefresh-run-cf2507.yaml +# codefresh create step-type -f /tmp/helm-cf2507.yaml +# 3. Replace __ACCOUNT__ in this file with your account name and create a pipeline with this YAML (no Git trigger is needed). +# 4. Pipeline variables: +# CF2507_HELM_VERSION (required) tag of quay.io/codefreshplugins/cfstep-helm to test, e.g. 3.14.0-- +# CF2507_HELM_DEBIAN_VERSION (required) tag of quay.io/codefreshplugins/cfstep-helm-debian to test +# CF2507_KUBE_CONTEXT (optional) name of a connected cluster; enables real `helm upgrade --install --dry-run` checks. +# Nothing is installed: the release is rendered with --dry-run only. +# 5. Run the pipeline. Expected result: all steps green. +# +# Negative check (proves the tests catch the vulnerability): switch the step types to the released vulnerable +# versions (codefresh-run:1.6.0, helm:1.1.12) and the helm image tags to the current released images. +# The "verify_*" steps must then fail and list the created markers. +# +# Manual check: in the log of run_child_with_values the "Executing command" line must show "$CFRUN_SECRET_0" +# and must not show the secret value. +# +# Legacy mode: the *_legacy steps enable the temporary legacy_shell_expansion option and check that it restores the +# previous behaviour for benign values ($VAR and $(...) are evaluated) and prints the migration warning. +# The default steps check that the option is disabled by default. + +version: '1.0' + +indicators: + - parentOnly: &parentOnly + condition: + all: + parentMode: 'includes("${{CF2507_MODE}}", "{{CF2507_MODE}}") == true' + - childOnly: &childOnly + condition: + all: + childMode: '"${{CF2507_MODE}}" == "child"' + - childLegacyOnly: &childLegacyOnly + condition: + all: + childMode: '"${{CF2507_MODE}}" == "child-legacy"' + - clusterOnly: &clusterOnly + condition: + all: + parentMode: 'includes("${{CF2507_MODE}}", "{{CF2507_MODE}}") == true' + hasCluster: 'includes("${{CF2507_KUBE_CONTEXT}}", "{{CF2507_KUBE_CONTEXT}}") == false' + + # Values passed to cfstep-helm in DRY_RUN mode. With DRY_RUN every helm/kubectl command is printed with echo, + # so the output shows exactly which arguments Helm would receive. + - environment: &helmDryRunEnv + - DRY_RUN=true + - ACTION=install + - SKIP_CF_STABLE_HELM_REPO=true + - CHART_REF=./chart + - KUBE_CONTEXT=$(touch /codefresh/volume/cf2507-markers/helm-kube-context) + - RELEASE_NAME=r;touch /codefresh/volume/cf2507-markers/helm-release + - NAMESPACE=`touch /codefresh/volume/cf2507-markers/helm-namespace` + - CHART_VERSION=1.0.0|touch /codefresh/volume/cf2507-markers/helm-chart-version + - CHART_REPO_URL=https://charts.example.com/$(touch /codefresh/volume/cf2507-markers/helm-repo-url) + - TIMEOUT=5m && touch /codefresh/volume/cf2507-markers/helm-timeout + - SET_FILE=cfg=$(touch /codefresh/volume/cf2507-markers/helm-set-file) + - 'CMD_PS=--atomic --description "$(touch /codefresh/volume/cf2507-markers/helm-cmd-ps)" ; touch /codefresh/volume/cf2507-markers/helm-cmd-ps-2' + - CUSTOM_image_tag=$(touch /codefresh/volume/cf2507-markers/helm-set) + - "VALUESTRING_image_name=x' && touch /codefresh/volume/cf2507-markers/helm-set-string && echo '" + - CUSTOMFILE_0=values.yaml;touch /codefresh/volume/cf2507-markers/helm-values-file + - CF_CTX_cf2507_URL=https://repo.example.com/$(touch /codefresh/volume/cf2507-markers/helm-ctx-repo) + - 'COMMIT_MESSAGE=x/e;touch /codefresh/volume/cf2507-markers/helm-commit-sed $(touch /codefresh/volume/cf2507-markers/helm-commit)' + # Backward compatibility: quote and backslash removal must keep working as before + - 'CUSTOM_pat_arr="{one,two,three}"' + - 'CUSTOM_STR__WITH__COMAS="one\,two\,three"' + - VALUESTRING_spaced=a b + + - commands: &helmDryRunCommands + - | + cd /codefresh/volume/cf2507 + printf 'Release notes\ncommit_message:previous\n' > chart/templates/NOTES.txt + rc=0 + bash /opt/bin/release_chart > helm-out.txt 2>&1 || rc=$? + cat helm-out.txt + if [ "$rc" != 0 ]; then echo "FAIL: release_chart exited with $rc"; exit 1; fi + if grep -q 'legacy_shell_expansion is enabled' helm-out.txt; then echo "FAIL: legacy mode must be disabled by default"; exit 1; fi + sh ./assert-helm-dry-run.sh helm-out.txt + + # Real Helm against a cluster, without installing anything (--dry-run) + - environment: &helmClusterEnv + - ACTION=install + - KUBE_CONTEXT=${{CF2507_KUBE_CONTEXT}} + - KUBECONFIG=${{CF_KUBECONFIG_PATH}} + - SKIP_CF_STABLE_HELM_REPO=true + - CHART_REF=./chart + - RELEASE_NAME=cf2507-test + - NAMESPACE=default + - CMD_PS=--dry-run --output json --description "cf2507 test" + - CUSTOM_image_tag=$(touch /codefresh/volume/cf2507-markers/helm-cluster-set) + - 'CUSTOM_pat_arr="{one,two,three}"' + - 'CUSTOM_STR__WITH__COMAS="one\,two\,three"' + - VALUESTRING_spaced=a b + - VALUESTRING_version=1.10 + + - commands: &helmClusterCommands + - | + cd /codefresh/volume/cf2507 + rc=0 + bash /opt/bin/release_chart > helm-cluster-out.txt 2>&1 || rc=$? + cat helm-cluster-out.txt + if [ "$rc" != 0 ]; then echo "FAIL: release_chart exited with $rc"; exit 1; fi + python3 ./verify-helm-cluster.py helm-cluster-out.txt + + # Legacy mode with benign values: the shell must evaluate $VAR and $(...) like before the fix + - environment: &helmLegacyEnv + - DRY_RUN=true + - ACTION=install + - LEGACY_SHELL_EXPANSION=true + - SKIP_CF_STABLE_HELM_REPO=true + - CHART_REF=./chart + - KUBE_CONTEXT=local + - RELEASE_NAME=cf2507-legacy + - MY_TAG=1.2.3 + - CUSTOM_image_tag=$MY_TAG + - CMD_PS=--description $(echo legacy-ok) + + - commands: &helmLegacyCommands + - | + cd /codefresh/volume/cf2507 + rc=0 + bash /opt/bin/release_chart > helm-legacy-out.txt 2>&1 || rc=$? + cat helm-legacy-out.txt + if [ "$rc" != 0 ]; then echo "FAIL: release_chart exited with $rc"; exit 1; fi + grep -qxF 'helm upgrade cf2507-legacy ./chart --install --reset-values --set image.tag=1.2.3 --description legacy-ok' helm-legacy-out.txt || { echo "FAIL: legacy mode didn't evaluate the values"; exit 1; } + grep -qF 'WARNING: legacy_shell_expansion is enabled' helm-legacy-out.txt || { echo "FAIL: legacy mode warning is missing"; exit 1; } + grep -qF 'behave differently once legacy_shell_expansion is disabled: CMD_PS, CUSTOM_image_tag' helm-legacy-out.txt || { echo "FAIL: legacy mode warning doesn't list the values"; exit 1; } + echo "OK: legacy mode evaluates values like before and prints the warning" + + - commands: &verifyMarkersCommands + - | + if [ -n "$(ls -A /codefresh/volume/cf2507-markers 2>/dev/null)" ]; then + echo "FAIL: injected commands were executed, markers:" + ls -1 /codefresh/volume/cf2507-markers + exit 1 + fi + echo "OK: no injected command was executed" + +stages: + - prepare + - codefresh-run + - helm-image + - helm-step + - helm-cluster + - child + +steps: + + # ---------------------------------------------------------------- prepare + prepare: + title: Prepare test chart and helper scripts + stage: prepare + image: alpine:3.20 + when: *parentOnly + commands: + - | + rm -rf /codefresh/volume/cf2507 /codefresh/volume/cf2507-markers + mkdir -p /codefresh/volume/cf2507/chart/templates /codefresh/volume/cf2507-markers + cd /codefresh/volume/cf2507 + + cat > chart/Chart.yaml <<'EOF' + apiVersion: v2 + name: cf2507 + version: 1.0.0 + EOF + + cat > chart/values.yaml <<'EOF' + {} + EOF + + cat > chart/templates/configmap.yaml <<'EOF' + apiVersion: v1 + kind: ConfigMap + metadata: + name: {{ .Release.Name }} + data: + values.json: {{ toJson .Values | quote }} + EOF + + # Arguments Helm must receive in DRY_RUN mode (printed by echo, one command per line) + cat > expected-helm-dry-run.txt <<'EOF' + kubectl config use-context $(touch /codefresh/volume/cf2507-markers/helm-kube-context) + helm repo add cf2507 https://repo.example.com/$(touch /codefresh/volume/cf2507-markers/helm-ctx-repo)/ + helm upgrade r;touch /codefresh/volume/cf2507-markers/helm-release ./chart --install --reset-values --repo https://charts.example.com/$(touch /codefresh/volume/cf2507-markers/helm-repo-url)/ --version 1.0.0|touch /codefresh/volume/cf2507-markers/helm-chart-version --namespace `touch /codefresh/volume/cf2507-markers/helm-namespace` --values values.yaml;touch /codefresh/volume/cf2507-markers/helm-values-file --set STR_WITH_COMAS=one\,two\,three --set image.tag=$(touch /codefresh/volume/cf2507-markers/helm-set) --set pat.arr={one,two,three} --set-string image.name=x && touch /codefresh/volume/cf2507-markers/helm-set-string && echo --set-string spaced=a b --timeout 5m && touch /codefresh/volume/cf2507-markers/helm-timeout --atomic --description $(touch /codefresh/volume/cf2507-markers/helm-cmd-ps) ; touch /codefresh/volume/cf2507-markers/helm-cmd-ps-2 --set-file cfg=$(touch /codefresh/volume/cf2507-markers/helm-set-file) + EOF + + cat > expected-notes.txt <<'EOF' + Release notes + commit_message:x/e;touch /codefresh/volume/cf2507-markers/helm-commit-sed $(touch /codefresh/volume/cf2507-markers/helm-commit) + EOF + + cat > assert-helm-dry-run.sh <<'EOF' + failed=0 + while IFS= read -r line; do + if grep -qxF -- "$line" "$1"; then + echo "OK: $line" + else + echo "FAIL: expected line not found: $line" + failed=1 + fi + done < /codefresh/volume/cf2507/expected-helm-dry-run.txt + if cmp -s /codefresh/volume/cf2507/expected-notes.txt /codefresh/volume/cf2507/chart/templates/NOTES.txt; then + echo "OK: commit message written literally to NOTES.txt" + else + echo "FAIL: unexpected NOTES.txt content:" + cat /codefresh/volume/cf2507/chart/templates/NOTES.txt + failed=1 + fi + exit $failed + EOF + + cat > dump-env.py <<'EOF' + import json, os, sys + with open(sys.argv[1], 'w') as f: + json.dump(dict(os.environ), f, indent=2, sort_keys=True) + EOF + + cat > verify-helm-step-env.py <<'EOF' + # Checks the environment the helm step template passed to the container + # Known issue (not related to CF-2507): in the debian image /bin/sh is dash, which drops environment + # variables with dots in the name (CUSTOM_pat.arr), so custom_values never reach the step there. + # With --allow-missing-dotted such variables are reported as SKIP when missing (they must still match when present). + import json, sys + env = json.load(open(sys.argv[1])) + allow_missing_dotted = '--allow-missing-dotted' in sys.argv[2:] + expected = { + 'NAMESPACE': "ns'\nprivileged: true", + 'RELEASE_NAME': 'r";$(touch /codefresh/volume/cf2507-markers/helm-step-release)', + 'CHART_VERSION': "1.0.0'\n - INJECTED_CHART_VERSION=1", + 'CUSTOM_pat.arr': '"{one,two,three}"', + 'CUSTOM_image.tag': "a'\n - INJECTED_CUSTOM=1", + 'CUSTOMFILE_0': "f'\n - INJECTED_FILE=1", + 'CF_CTX_cf2507_URL': "https://repo.example.com/x'\n - INJECTED_REPO=1", + 'SKIP_CF_STABLE_HELM_REPO': 'true', + 'ACTION': 'auth', + 'LEGACY_SHELL_EXPANSION': 'false', + } + failed = False + for key, value in expected.items(): + if env.get(key) == value: + print('OK: %s' % key) + elif allow_missing_dotted and '.' in key and key not in env: + print('SKIP: %s is missing (known dash issue, see above)' % key) + else: + print('FAIL: %s: got %r, expected %r' % (key, env.get(key), value)) + failed = True + injected = sorted(k for k in env if k.startswith('INJECTED')) + if injected: + print('FAIL: YAML injection created environment variables: %s' % injected) + failed = True + sys.exit(1 if failed else 0) + EOF + + cat > verify-helm-cluster.py <<'EOF' + # Parses `helm upgrade --output json` and checks the values Helm received + import json, sys + release = None + for line in open(sys.argv[1]): + if line.startswith('{') and '"manifest"' in line: + release = json.loads(line) + if release is None: + print('FAIL: helm release JSON not found in the output') + sys.exit(1) + expected = { + 'STR_WITH_COMAS': 'one,two,three', + 'image': {'tag': '$(touch /codefresh/volume/cf2507-markers/helm-cluster-set)'}, + 'pat': {'arr': ['one', 'two', 'three']}, + 'spaced': 'a b', + 'version': '1.10', + } + if release.get('config') != expected: + print('FAIL: unexpected values: %s' % json.dumps(release.get('config'), sort_keys=True)) + print('expected: %s' % json.dumps(expected, sort_keys=True)) + sys.exit(1) + if release.get('info', {}).get('description') != 'cf2507 test': + print('FAIL: unexpected description: %r' % release.get('info', {}).get('description')) + sys.exit(1) + print('OK: Helm received the expected values') + EOF + + ls -la /codefresh/volume/cf2507 + + # ---------------------------------------------------------------- codefresh-run + run_child_with_values: + title: codefresh-run - values reach the child pipeline literally + stage: codefresh-run + type: __ACCOUNT__/codefresh-run-cf2507 + when: *parentOnly + arguments: + PIPELINE_ID: '${{CF_PIPELINE_NAME}}' + DETACH: false + FOLLOW_LOGS: true + TIMEOUT_MINS: '15' + VARIABLE: + - 'CF2507_MODE=child' + - 'V_SPACES=value with spaces' + - 'V_COLON=key: value' + - 'V_QUOTES=it''s "quoted"' + - 'V_DOLLAR=$HOME' + - 'V_SUBST=$(touch /codefresh/volume/cf2507-markers/run-variable)' + - 'V_BACKTICK=`touch /codefresh/volume/cf2507-markers/run-backtick`' + - 'V_SEMI=x;touch /codefresh/volume/cf2507-markers/run-semicolon' + SECRETS: + - 'S_TOKEN=s3cr3t$(touch /codefresh/volume/cf2507-markers/run-secret)' + + run_injection_arguments: + title: codefresh-run - payloads in every string argument + stage: codefresh-run + type: __ACCOUNT__/codefresh-run-cf2507 + when: *parentOnly + # The CLI call itself is expected to fail (the trigger, branch and runtime don't exist) + fail_fast: false + arguments: + PIPELINE_ID: '${{CF_PIPELINE_NAME}}" ; touch /codefresh/volume/cf2507-markers/run-pipeline-id ; echo "' + TRIGGER_ID: 'trigger;touch /codefresh/volume/cf2507-markers/run-trigger' + BRANCH: '$(touch${IFS}/codefresh/volume/cf2507-markers/run-branch)' + SHA: '`touch /codefresh/volume/cf2507-markers/run-sha`' + RUNTIME_NAME: 'runtime && touch /codefresh/volume/cf2507-markers/run-runtime' + PACK_NAME: 'small|touch /codefresh/volume/cf2507-markers/run-pack' + VARIABLE_FILE: 'vars.yml;touch /codefresh/volume/cf2507-markers/run-var-file' + YAML: 'codefresh.yml;touch /codefresh/volume/cf2507-markers/run-yaml' + CONTEXT: + - 'ctx;touch /codefresh/volume/cf2507-markers/run-context' + ONLY: + - 'step;touch /codefresh/volume/cf2507-markers/run-only' + SKIP: + - 'step;touch /codefresh/volume/cf2507-markers/run-skip' + DETACH: true + + run_injection_timeout: + title: codefresh-run - payload in TIMEOUT_MINS + stage: codefresh-run + type: __ACCOUNT__/codefresh-run-cf2507 + when: *parentOnly + # `codefresh wait` is expected to reject the invalid timeout + fail_fast: false + arguments: + PIPELINE_ID: '${{CF_PIPELINE_NAME}}' + FOLLOW_LOGS: false + TIMEOUT_MINS: '10;touch /codefresh/volume/cf2507-markers/run-timeout' + VARIABLE: + - 'CF2507_MODE=noop' + + run_child_legacy: + title: codefresh-run - LEGACY_SHELL_EXPANSION evaluates values like before + stage: codefresh-run + type: __ACCOUNT__/codefresh-run-cf2507 + when: *parentOnly + arguments: + PIPELINE_ID: '${{CF_PIPELINE_NAME}}' + LEGACY_SHELL_EXPANSION: true + DETACH: false + FOLLOW_LOGS: true + TIMEOUT_MINS: '15' + VARIABLE: + - 'CF2507_MODE=child-legacy' + - 'V_LEGACY=$(echo legacy-ok)' + + verify_codefresh_run: + title: codefresh-run - no injected command was executed + stage: codefresh-run + image: alpine:3.20 + when: *parentOnly + commands: *verifyMarkersCommands + + # ---------------------------------------------------------------- cfstep-helm image + helm_image_alpine: + title: cfstep-helm image - generated script (alpine) + stage: helm-image + image: quay.io/codefreshplugins/cfstep-helm:${{CF2507_HELM_VERSION}} + when: *parentOnly + environment: *helmDryRunEnv + commands: *helmDryRunCommands + + verify_helm_image_alpine: + title: cfstep-helm image - no injected command was executed (alpine) + stage: helm-image + image: alpine:3.20 + when: *parentOnly + commands: *verifyMarkersCommands + + helm_image_debian: + title: cfstep-helm image - generated script (debian) + stage: helm-image + image: quay.io/codefreshplugins/cfstep-helm-debian:${{CF2507_HELM_DEBIAN_VERSION}} + when: *parentOnly + environment: *helmDryRunEnv + commands: *helmDryRunCommands + + verify_helm_image_debian: + title: cfstep-helm image - no injected command was executed (debian) + stage: helm-image + image: alpine:3.20 + when: *parentOnly + commands: *verifyMarkersCommands + + helm_image_legacy_alpine: + title: cfstep-helm image - legacy_shell_expansion evaluates values like before (alpine) + stage: helm-image + image: quay.io/codefreshplugins/cfstep-helm:${{CF2507_HELM_VERSION}} + when: *parentOnly + environment: *helmLegacyEnv + commands: *helmLegacyCommands + + helm_image_legacy_debian: + title: cfstep-helm image - legacy_shell_expansion evaluates values like before (debian) + stage: helm-image + image: quay.io/codefreshplugins/cfstep-helm-debian:${{CF2507_HELM_DEBIAN_VERSION}} + when: *parentOnly + environment: *helmLegacyEnv + commands: *helmLegacyCommands + + # ---------------------------------------------------------------- helm step template + # The auth action without kube_context doesn't need a cluster; the commands dump the container environment + helm_step_alpine: + title: helm step - argument values stay inside YAML strings (alpine) + stage: helm-step + type: __ACCOUNT__/helm-cf2507 + when: *parentOnly + arguments: + action: auth + helm_version: '${{CF2507_HELM_VERSION}}' + skip_cf_stable_helm_repo: true + namespace: "ns'\nprivileged: true" + release_name: 'r";$(touch /codefresh/volume/cf2507-markers/helm-step-release)' + chart_version: "1.0.0'\n - INJECTED_CHART_VERSION=1" + custom_values: + - 'pat.arr="{one,two,three}"' + - "image.tag=a'\n - INJECTED_CUSTOM=1" + custom_value_files: + - "f'\n - INJECTED_FILE=1" + repos: + - name: cf2507 + url: "https://repo.example.com/x'\n - INJECTED_REPO=1" + commands: + - python3 /codefresh/volume/cf2507/dump-env.py /codefresh/volume/cf2507/helm-step-alpine.json + + verify_helm_step_alpine: + title: helm step - verify container environment (alpine) + stage: helm-step + image: quay.io/codefreshplugins/cfstep-helm:${{CF2507_HELM_VERSION}} + when: *parentOnly + commands: + - python3 /codefresh/volume/cf2507/verify-helm-step-env.py /codefresh/volume/cf2507/helm-step-alpine.json + + helm_step_debian: + title: helm step - argument values stay inside YAML strings (debian) + stage: helm-step + type: __ACCOUNT__/helm-cf2507 + when: *parentOnly + arguments: + action: auth + use_debian_image: true + helm_version: '${{CF2507_HELM_DEBIAN_VERSION}}' + skip_cf_stable_helm_repo: true + namespace: "ns'\nprivileged: true" + release_name: 'r";$(touch /codefresh/volume/cf2507-markers/helm-step-release)' + chart_version: "1.0.0'\n - INJECTED_CHART_VERSION=1" + custom_values: + - 'pat.arr="{one,two,three}"' + - "image.tag=a'\n - INJECTED_CUSTOM=1" + custom_value_files: + - "f'\n - INJECTED_FILE=1" + repos: + - name: cf2507 + url: "https://repo.example.com/x'\n - INJECTED_REPO=1" + commands: + - python3 /codefresh/volume/cf2507/dump-env.py /codefresh/volume/cf2507/helm-step-debian.json + + verify_helm_step_debian: + title: helm step - verify container environment (debian) + stage: helm-step + image: quay.io/codefreshplugins/cfstep-helm:${{CF2507_HELM_VERSION}} + when: *parentOnly + commands: + - python3 /codefresh/volume/cf2507/verify-helm-step-env.py /codefresh/volume/cf2507/helm-step-debian.json --allow-missing-dotted + + verify_helm_step_markers: + title: helm step - no injected command was executed + stage: helm-step + image: alpine:3.20 + when: *parentOnly + commands: *verifyMarkersCommands + + # ---------------------------------------------------------------- real Helm (optional) + helm_cluster_alpine: + title: Real Helm receives the expected values (alpine) + stage: helm-cluster + image: quay.io/codefreshplugins/cfstep-helm:${{CF2507_HELM_VERSION}} + when: *clusterOnly + environment: *helmClusterEnv + commands: *helmClusterCommands + + helm_cluster_debian: + title: Real Helm receives the expected values (debian) + stage: helm-cluster + image: quay.io/codefreshplugins/cfstep-helm-debian:${{CF2507_HELM_DEBIAN_VERSION}} + when: *clusterOnly + environment: *helmClusterEnv + commands: *helmClusterCommands + + verify_helm_cluster: + title: Real Helm - no injected command was executed + stage: helm-cluster + image: alpine:3.20 + when: *clusterOnly + commands: *verifyMarkersCommands + + # ---------------------------------------------------------------- child mode (started by run_child_with_values) + child_verify_variables: + title: Child - variables from codefresh-run arrived unchanged + stage: child + image: alpine:3.20 + when: *childOnly + environment: + - GOT_SPACES=${{V_SPACES}} + - GOT_COLON=${{V_COLON}} + - GOT_QUOTES=${{V_QUOTES}} + - GOT_DOLLAR=${{V_DOLLAR}} + - GOT_SUBST=${{V_SUBST}} + - GOT_BACKTICK=${{V_BACKTICK}} + - GOT_SEMI=${{V_SEMI}} + - GOT_SECRET=${{S_TOKEN}} + commands: + - | + failed=0 + check() { + if [ "$1" = "$2" ]; then echo "OK: $3"; else echo "FAIL: $3: got [$1], expected [$2]"; failed=1; fi + } + check "$GOT_SPACES" 'value with spaces' V_SPACES + check "$GOT_COLON" 'key: value' V_COLON + check "$GOT_QUOTES" 'it'\''s "quoted"' V_QUOTES + check "$GOT_DOLLAR" '$HOME' V_DOLLAR + check "$GOT_SUBST" '$(touch /codefresh/volume/cf2507-markers/run-variable)' V_SUBST + check "$GOT_BACKTICK" '`touch /codefresh/volume/cf2507-markers/run-backtick`' V_BACKTICK + check "$GOT_SEMI" 'x;touch /codefresh/volume/cf2507-markers/run-semicolon' V_SEMI + check "$GOT_SECRET" 's3cr3t$(touch /codefresh/volume/cf2507-markers/run-secret)' S_TOKEN + exit $failed + + child_verify_legacy: + title: Child - LEGACY_SHELL_EXPANSION evaluated the value in the parent + stage: child + image: alpine:3.20 + when: *childLegacyOnly + environment: + - GOT_LEGACY=${{V_LEGACY}} + commands: + - | + if [ "$GOT_LEGACY" = 'legacy-ok' ]; then echo "OK: V_LEGACY"; else echo "FAIL: V_LEGACY: got [$GOT_LEGACY], expected [legacy-ok]"; exit 1; fi \ No newline at end of file diff --git a/tests/cf2507-pr-descriptions.md b/tests/cf2507-pr-descriptions.md new file mode 100644 index 000000000..ccafa1830 --- /dev/null +++ b/tests/cf2507-pr-descriptions.md @@ -0,0 +1,199 @@ +# CF-2507 PR descriptions + +Three PRs, one per branch. `steps` and `cfstep-helm` are public repositories, so the descriptions explain the +class of the issue and the fix, but intentionally contain no working exploit or details from the pentest report. + +--- + +## 1. codefresh-io/steps — `CF-2507-fix-interpolation-security` → `master` + +**Title:** fix(CF-2507): prevent shell injection in codefresh-run and helm steps + +### Summary + +The `codefresh-run` step template interpolated raw argument values into a shell command. Arguments are often +derived from Git data (the documented example uses `BRANCH: '${{CF_BRANCH}}'`), so a crafted value, for example +a branch name, could execute arbitrary commands in the pipeline build container. + +The `helm` step template rendered argument values inside single-quoted YAML strings, so a quote or a new line in a +value could add arbitrary keys to the generated step. + +### Changes + +**codefresh-run 1.6.0 → 1.7.0** +- All arguments are passed to the step containers as environment variables (`CFRUN_*`) and are referenced in the + command only as `"$CFRUN_*"`. The command line contains no user input anymore. +- Array arguments (`VARIABLE`, `SECRETS`, `CONTEXT`, `ONLY`, `SKIP`) are passed as indexed variables + (`CFRUN_VARIABLE_0`, ...). +- `TIMEOUT_MINS` in the second step goes through the environment as well. +- Environment entries and the `image` are rendered with `toJSON`, so values can't break out of the YAML string. +- Side effect: secret values are no longer printed in the "Executing command" log line. + +**helm 1.1.12 → 1.1.13** +- Environment entries and the `image` are rendered with `toJSON` instead of `'...'`. +- `cmd_ps` documentation updated (schema description and README). +- The escaping of the generated Helm script is fixed in the `cfstep-helm` image (see related PRs). + +**Temporary migration option (both steps)** +- `codefresh-run`: `LEGACY_SHELL_EXPANSION` (boolean, default `false`). When enabled, the arguments are interpolated + exactly as in 1.6.0, and the step prints a warning with the names (not the values) of the arguments that contain + shell syntax. +- `helm`: `legacy_shell_expansion` (boolean, default `false`), passed to the image as `LEGACY_SHELL_EXPANSION`. +- The option is unsafe with user-controlled values. It only gives users time to migrate pipelines that rely on shell + expansion, and will be removed in a later version. + +### Behavior changes + +Argument values are now passed to the CLI literally: +- `$VAR`, `$(...)`, backticks, globs and `~` in arguments are no longer expanded by the shell. + Use `${{VAR}}` instead of `$VAR`; it is substituted by the engine before the step runs, as before. +- Several flags can't be passed through a single argument anymore (e.g. `BRANCH: "main --no-cache"`). +- Values with spaces now work as a single argument (`VARIABLE: ["key=value with spaces"]` was split before). + +Pipelines that need time to migrate can set `LEGACY_SHELL_EXPANSION: true` (`codefresh-run`) or +`legacy_shell_expansion: true` (`helm`) to get the previous behavior back temporarily. + +Pipelines that pin `codefresh-run:1.6.0` or older are not affected by this PR and should be upgraded. Pinned `helm` +step versions get the new behavior from the rebuilt `cfstep-helm` images; they can't set the new argument, so they +need to upgrade to `helm:1.1.13` to use the migration option. + +### Testing + +- Rendered both templates with the production template engine (pikolo 0.15.16): + - benign arguments: the rendered `helm` step is identical to the previous version; `codefresh run` receives the + same arguments as before, except values with spaces, which are now passed correctly; + - malicious arguments: the previous templates produced broken YAML or executed the injected commands, the new + ones keep every value as a literal string. +- Executed the rendered `codefresh-run` commands in `quay.io/codefresh/cli:0.89.1` with a stubbed CLI the same way + the engine does (`/bin/sh -c`, `set -e`): no injected command was executed, all values reached the CLI unchanged. +- `LEGACY_SHELL_EXPANSION: true`: the CLI receives exactly the same arguments as with `codefresh-run` 1.6.0 + (including `$VAR` and `$(...)` values), and the warning lists the arguments with shell syntax. With the default + value the rendered steps are identical to the version without the option. +- `tests/cf2507-interpolation-security.yml`: verification pipeline covering all three PRs (not committed yet; + it hasn't been run in Codefresh yet). + +### Related PRs + +- codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security` (alpine images) +- codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security-debian` (debian images) + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +--- + +## 2. codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security` → `master` + +**Title:** fix(CF-2507): prevent shell injection in generated helm script + +### Summary + +`EntrypointScriptBuilder` generates `/tmp/run` by interpolating environment values (`KUBE_CONTEXT`, `RELEASE_NAME`, +`NAMESPACE`, `custom_values`, `CMD_PS`, repository URLs, ...) into shell commands without escaping. When a pipeline +passes Git-derived data to the helm step, a crafted value could execute arbitrary commands in the build container. +`COMMIT_MESSAGE` was additionally interpolated into a `sed` expression, which allowed command execution through +the `e` flag of GNU sed. + +### Changes + +- Every value interpolated into the generated script is quoted with `shlex.quote`. +- `--set` / `--set-string` values are parsed with `shlex.split` before quoting. This keeps the existing quote and + backslash removal (e.g. `pat.arr="{one,two,three}"`, `one\,two\,three`) without performing any expansion. + A value that is not a single shell word is passed literally. +- `CMD_PS` is split and re-quoted token by token: quotes are respected, but `$VAR`, `$(...)` and shell operators are + passed to Helm literally. Unbalanced quotes fail with `Failed to parse CMD_PS`. +- `COMMIT_MESSAGE` is passed to `awk` through the environment instead of being interpolated into shell/sed code. + Multi-line commit messages now work (they broke `sed` before). +- `$PACKAGE`, `$HELMREPO_USERNAME:$HELMREPO_PASSWORD` and `$GOOGLE_APPLICATION_CREDENTIALS_JSON` are quoted. +- Temporary migration option `LEGACY_SHELL_EXPANSION` (`legacy_shell_expansion` step argument), disabled by + default. An unset variable, including all older `helm` step versions, uses the safe behavior. When enabled, values + provided by the user are interpolated as before, and a warning lists the names (not the values) of the variables + that contain shell syntax. `COMMIT_MESSAGE` and internal variables stay escaped in both modes. +- Tests: new tests execute the generated script with stubbed `helm`/`kubectl` and assert that no injected command is + executed and that Helm receives the expected arguments. `assertEquals` replaced with `assertEqual` (removed in + Python 3.12, two tests were failing before this change). + +### Behavior changes + +- `$VAR`, `$(...)`, globs and `~` in values and in `CMD_PS` are no longer expanded, unless `LEGACY_SHELL_EXPANSION` + is enabled. +- Values with `;` or spaces in `--set-string`, and values like `it's`, now work (they broke the command before). +- The script printed in the log ("Running the following script") now contains quotes. Credential masking still works. + +The image tags stay the same (`cfstep-helm:`), so after the images are rebuilt the fix applies to all +versions of the `helm` step, including the ones pinned to older step versions. + +### Testing + +- `python3 -m unittest tests/EntrypointScriptBuilderTest.py`: 26/26 locally and inside + `quay.io/codefreshplugins/cfstep-helm:3.14.0` (Python 3.9, busybox awk). +- The new tests fail against `origin/master` (the injected commands are executed). +- With `LEGACY_SHELL_EXPANSION=true` Helm receives exactly the same arguments as with `origin/master`, including + `$VAR` values and globs. +- Compared the arguments Helm receives before and after the change for typical values: identical, except for the + cases that were broken before (listed above). +- Ran the image in `DRY_RUN` mode with malicious values for every input: no command was executed, Helm received + the values literally. +- Not run: robot acceptance tests (they need real `helm`/`kubectl` binaries). + +### Related PRs + +- codefresh-io/steps — `CF-2507-fix-interpolation-security` (helm 1.1.13, codefresh-run 1.7.0) +- codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security-debian` (same fix for the debian images) + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +--- + +## 3. codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security-debian` → `debian` + +**Title:** fix(CF-2507): prevent shell injection in generated helm script (debian) + +### Summary + +Port of the master fix to the `debian` branch, which the `cfstep-helm-debian` images (`use_debian_image: true`) are +built from. Without it the debian images stay vulnerable after the rebuild. + +`EntrypointScriptBuilder` generates `/tmp/run` by interpolating environment values into shell commands without +escaping, and `COMMIT_MESSAGE` was interpolated into a `sed` expression. When a pipeline passes Git-derived data to +the helm step, a crafted value could execute arbitrary commands in the build container. + +### Changes + +Same as the master PR: +- every value interpolated into the generated script is quoted with `shlex.quote`; +- `--set` / `--set-string` values keep the existing quote and backslash removal without expansions; +- `CMD_PS` is re-quoted token by token; +- `COMMIT_MESSAGE` is passed to `awk` through the environment; +- `$PACKAGE`, Helm repository credentials and `$GOOGLE_APPLICATION_CREDENTIALS_JSON` are quoted; +- the same temporary `LEGACY_SHELL_EXPANSION` migration option, disabled by default; +- the same tests as in the master PR. + +The `debian` branch has an older `EntrypointScriptBuilder` (no per-repository credentials, no +`use_repos_for_auth_action`). The conflict in the `helm repo add` block was resolved by keeping the debian logic and +adding the escaping only. No functionality from master was ported. + +### Behavior changes + +Same as the master PR: `$VAR`, `$(...)`, globs and `~` in values and in `CMD_PS` are no longer expanded, unless +`LEGACY_SHELL_EXPANSION` is enabled. + +### Testing + +- `python3 -m unittest tests/EntrypointScriptBuilderTest.py`: 26/26 locally and inside + `quay.io/codefreshplugins/cfstep-helm-debian:3.14.0` (Python 3.8, mawk). +- The new tests fail against `origin/debian` (the injected commands are executed). +- With `LEGACY_SHELL_EXPANSION=true` Helm receives exactly the same arguments as with `origin/debian`. +- Ran the image in `DRY_RUN` mode with malicious values for every input: no command was executed. + +### Known issue (not related to this PR) + +In the debian image `/bin/sh` is `dash`, and the engine runs step commands with `/bin/sh -c`. `dash` drops +environment variables with dots in the name, so `custom_values` (`CUSTOM_`) never reach the step when +`use_debian_image: true` is set. This happens before and after this change; it should be tracked separately. + +### Related PRs + +- codefresh-io/steps — `CF-2507-fix-interpolation-security` (helm 1.1.13, codefresh-run 1.7.0) +- codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security` (same fix for master / alpine images) + +🤖 Generated with [Claude Code](https://claude.com/claude-code) \ No newline at end of file diff --git a/tests/cf2507-security-advisory.md b/tests/cf2507-security-advisory.md new file mode 100644 index 000000000..d54b066a0 --- /dev/null +++ b/tests/cf2507-security-advisory.md @@ -0,0 +1,133 @@ +# Security advisory: command injection in the codefresh-run and helm pipeline steps + + + +| | | +|---|---| +| **Advisory ID** | [TBD] | +| **CVE** | [TBD] | +| **Severity** | High | +| **Published** | [TBD] | +| **Affected products** | Codefresh pipelines (SaaS, hybrid, and on-premises) that use the `codefresh-run` or `helm` steps | +| **Internal reference** | CF-2507 | + +## Summary + +We found and fixed a command injection issue in two official Codefresh pipeline steps: **Run a Codefresh pipeline** (`codefresh-run`) and **Release a Helm chart** (`helm`). + +Both steps built shell commands from their arguments without escaping them. When a pipeline passes data that someone outside your team can control, such as a Git branch name or a commit message, to one of these arguments, that person can run their own commands in the pipeline build. They only need permission to push to a repository that triggers the pipeline. They don't need a Codefresh account. + +A successful attack runs with the permissions of the pipeline build. This includes access to the Codefresh API key of the user or service account that runs the pipeline, and to any secrets, registries, and clusters the build can reach. + +We aren't aware of this issue being exploited. We recommend that you upgrade as soon as possible. + +## Are you affected? + +You're affected if **all** of the following are true: + +1. A pipeline uses the `codefresh-run` step at version 1.6.0 or earlier, or the `helm` step at version 1.1.12 or earlier, or runs a `cfstep-helm` image directly as a freestyle step. +2. At least one step argument contains a value that someone outside your team can influence. Common examples: + - `${{CF_BRANCH}}`, `${{CF_COMMIT_MESSAGE}}`, `${{CF_PULL_REQUEST_TITLE}}`, or other `${{CF_PULL_REQUEST_*}}` variables + - variables derived from Git data, webhook payloads, or other external input +3. The pipeline runs automatically for that input, for example a Git trigger that runs on any branch (the default branch filter is `/.*/gi`). + +The documented `codefresh-run` example uses `BRANCH: '${{CF_BRANCH}}'`. If your pipelines follow this example and use a Git trigger, assume you're affected. + +| Step | Affected arguments | +|---|---| +| `codefresh-run` | `PIPELINE_ID`, `TRIGGER_ID`, `BRANCH`, `SHA`, `RUNTIME_NAME`, `PACK_NAME`, `VARIABLE`, `SECRETS`, `VARIABLE_FILE`, `CONTEXT`, `ONLY`, `SKIP`, `YAML`, and `TIMEOUT_MINS` | +| `helm` | All string arguments, including `kube_context`, `release_name`, `namespace`, `chart_name`, `chart_version`, `custom_values`, `custom_value_files`, `cmd_ps`, and repository URLs | + +## Fixed versions + +| Component | Fixed version | +|---|---| +| `codefresh-run` step | 1.7.0 | +| `helm` step | 1.1.13 | +| `quay.io/codefreshplugins/cfstep-helm` images | All tags rebuilt on [TBD] | +| `quay.io/codefreshplugins/cfstep-helm-debian` images | All tags rebuilt on [TBD] | + +The fixed steps pass argument values to the container as environment variables and never place them in a shell command. The `cfstep-helm` images escape every value they add to the generated Helm commands. + +## What you need to do + +### SaaS and hybrid runtimes + +1. **Pipelines that don't pin a step version** (`type: codefresh-run`, `type: helm`) use the fixed versions automatically. Check them for the [behavior changes](#behavior-changes) below. +2. **Pipelines that pin a step version** (for example `type: codefresh-run:1.6.0` or `type: helm:1.1.12`): update them to `codefresh-run:1.7.0` and `helm:1.1.13` or later, or remove the version to always use the latest one. +3. **Pipelines that run `cfstep-helm` images directly**: pull the image again. If you pin the image by digest, or copy it to your own registry, update the digest or mirror the rebuilt image. + +The rebuilt `cfstep-helm` images also protect older `helm` step versions from command injection. We still recommend upgrading to `helm` 1.1.13, because older versions of the step template can let a crafted value add unexpected settings to the generated step. + +### If a pipeline stops working after the upgrade + +If a pipeline relies on the shell evaluating argument values (see [Behavior changes](#behavior-changes)), you can temporarily get the previous behavior back while you update it: + +| Step | Setting | +|---|---| +| `codefresh-run` 1.7.0 or later | `LEGACY_SHELL_EXPANSION: true` | +| `helm` 1.1.13 or later | `legacy_shell_expansion: true` | +| `cfstep-helm` image used directly | environment variable `LEGACY_SHELL_EXPANSION=true` | + +When the option is enabled, the step log shows a warning with the names of the arguments that still use shell syntax. Update those arguments, then remove the option. + +> **Warning:** this option brings the vulnerability back for that step. Use it only for pipelines that don't receive values from outside your team, and only for as long as you need to migrate. We'll remove the option in a later version of the steps. + +Older `helm` step versions don't accept the new argument. To use it, upgrade the step to `helm` 1.1.13. + +### On-premises installations + +In addition to the steps above, make sure your installation uses the rebuilt `cfstep-helm` and `cfstep-helm-debian` images. If you mirror images to a private registry, mirror them again after [TBD]. + +### Check for signs of abuse + +If you were affected, review recent builds of pipelines that use these steps: + +- Look for builds triggered by branch names or commit messages that contain shell characters, such as `$(`, backticks, `;`, `|`, or `&&`. +- Look for unexpected network connections or commands in the logs of the `codefresh-run` and `helm` steps. + +If you find anything suspicious, rotate the Codefresh API keys of the users and service accounts that run those pipelines, and rotate any secrets the affected builds could access. + +## Behavior changes + +The fix changes how argument values are handled. Most pipelines keep working without changes. Check your pipelines if they rely on any of the following. Until you update them, you can use the [temporary option](#if-a-pipeline-stops-working-after-the-upgrade) to keep the previous behavior. + +- **Shell expansion in arguments.** `$VAR`, `$(...)`, backticks, wildcards (`*`), and `~` are now passed literally. Use Codefresh variables (`${{VAR}}`) instead. The engine replaces them before the step runs, the same way as before. Variables you export with `cf_export` are available as `${{VAR}}` in later steps. +- **Several flags in one `codefresh-run` argument.** For example, `BRANCH: "main --no-cache"` now passes the whole value as the branch name. Use the dedicated arguments, such as `NO_CACHE: true`, instead. +- **`cmd_ps` in the `helm` step.** Quotes still work as before (`--description "my release"`), but shell operators and expansions are passed to Helm literally. + +Some values that didn't work before now work correctly. These include `codefresh-run` variables with spaces, and `helm` values that contain `;`, spaces, or apostrophes. Quotes and escaped commas in `custom_values`, such as `pat.arr="{one,two,three}"` or `key="one\,two"`, keep working the same way. + +The `codefresh-run` step also no longer prints secret values in the "Executing command" line of the build log. + +## Workarounds + +If you can't upgrade right away: + +- Don't pass Git-derived values to the affected arguments. If you need a branch name, use `${{CF_BRANCH_TAG_NORMALIZED}}` instead of `${{CF_BRANCH}}`. It contains only characters that are valid in a Docker image tag, so it can't be used to inject commands. Don't rely on the `*_ESCAPED` variables for this: they aren't designed to make values safe for a shell. +- Restrict Git triggers to the branches you trust, instead of the default `/.*/gi` filter. +- Limit who can push to repositories that trigger pipelines with these steps. + +## Timeline + +| Date | Event | +|---|---| +| [TBD] | Issue reported during a security assessment | +| [TBD] | Fixed step versions released | +| [TBD] | All `cfstep-helm` images rebuilt | +| [TBD] | Advisory published | +| [TBD] | `LEGACY_SHELL_EXPANSION` / `legacy_shell_expansion` option removed | + +## Credit + +[TBD: confirm with the reporter whether and how they want to be credited.] + +## Questions + +If you have questions about this advisory, contact [Codefresh support](https://support.codefresh.io). \ No newline at end of file diff --git a/tests/cf2507-support-runbook.md b/tests/cf2507-support-runbook.md new file mode 100644 index 000000000..9a46e683a --- /dev/null +++ b/tests/cf2507-support-runbook.md @@ -0,0 +1,134 @@ +# Support runbook: codefresh-run and helm steps no longer evaluate shell syntax in arguments + + + +| | | +|---|---| +| **Released** | [TBD] | +| **Affected steps** | `codefresh-run` 1.7.0 and later; `helm` (all versions) after the `cfstep-helm` images were rebuilt on [TBD]; `cfstep-helm` / `cfstep-helm-debian` images used directly | +| **Temporary option removed on** | [TBD] | +| **Escalation** | [TBD: team / channel] | + +## What changed + +As a security fix, the `codefresh-run` and `helm` steps now use argument values exactly as written. Before, the values went through a shell, which evaluated `$VAR`, `$(...)`, backticks, wildcards (`*`), and `~`. Pipelines that relied on this get the literal text now, for example `$MY_TAG` instead of `1.2.3`. + +`${{VAR}}` (Codefresh variables) works exactly as before. + +## How to identify the issue + +It's most likely this issue if **all** of the following are true: + +1. The failing step is `codefresh-run` or `helm`, or a freestyle step that uses a `cfstep-helm` image. +2. The pipeline worked before [TBD release date] and its YAML didn't change. +3. A step argument contains `$` that isn't part of `${{...}}`, a backtick, `*`, or `~`. + +Typical symptoms: + +| Step | What the customer sees | +|---|---| +| `helm` | Values contain the literal text, e.g. image `my-repo:$MY_TAG` and `InvalidImageName` in Kubernetes | +| `helm` | `no context exists with the name: "$MY_CONTEXT"` | +| `helm` | `open values-*.yaml: no such file or directory` | +| `helm` | Helm complains about unexpected arguments when `cmd_ps` contains `;`, `&&`, or `\|` | +| `helm` | `Failed to parse CMD_PS: No closing quotation` (new error, `cmd_ps` has an unclosed quote) | +| `codefresh-run` | The child pipeline receives `$VAR` as a variable value instead of the real value | +| `codefresh-run` | The child pipeline isn't found, or the wrong branch is used, when `PIPELINE_ID` or `BRANCH` contains `$VAR` | +| `codefresh-run` | `codefresh wait` rejects the timeout when `TIMEOUT_MINS` contains `$VAR` | + +To confirm, open the step log: + +- **`helm`**: in the "Running the following script" section, the values appear in single quotes, e.g. `--set 'image.tag=$MY_TAG'`. +- **`codefresh-run`**: the "Executing command" line shows `"$CFRUN_..."` placeholders. Compare them with the step `arguments` in the pipeline YAML. + +### Not this issue + +- **`helm` with `use_debian_image: true` ignores `custom_values` written with dots in the key** (e.g. `image.tag=1.2.3`, no `$` involved). This is a separate known issue, CF-2559. Fix: use the documented format with underscores (`image_tag=1.2.3`); the step converts `_` to `.`. +- The step failed before [TBD release date] as well, or the pipeline YAML changed recently. + +## What to tell the customer + +Short explanation you can use as is: + +> We recently released a security update for the `codefresh-run` and `helm` steps. The steps now use argument values exactly as they're written, instead of passing them through a shell. Your pipeline uses shell syntax (`$VAR`) in a step argument, so the step now receives the text `$VAR` instead of its value. +> +> You can fix this by replacing the shell variables with Codefresh variables (`${{VAR}}`). If you need more time, you can temporarily restore the previous behavior with the option below. + +Don't share details about the vulnerability beyond what's in the published advisory: [TBD advisory link]. + +## Workaround: temporary legacy mode + +The customer adds one setting to the affected step: + +| Step | Setting | +|---|---| +| `codefresh-run` 1.7.0 or later | `LEGACY_SHELL_EXPANSION: true` | +| `helm` 1.1.13 or later | `legacy_shell_expansion: true` | +| `cfstep-helm` image used directly | environment variable `LEGACY_SHELL_EXPANSION=true` | + +```yaml +deploy: + type: helm:1.1.13 + arguments: + # ...existing arguments... + legacy_shell_expansion: true # Temporary +``` + +```yaml +run_child: + type: codefresh-run + arguments: + # ...existing arguments... + LEGACY_SHELL_EXPANSION: true # Temporary +``` + +Things to tell the customer: + +- **Older `helm` step versions** (`helm:1.1.12` and earlier) don't accept the new argument and fail with a validation error. Upgrade the step to `helm:1.1.13` first. +- **The option is less secure**: it brings back the behavior that the security update fixed. Recommend it only as a short-term fix, and only for pipelines that don't receive values from outside the customer's team (for example, branch names from a Git trigger). +- **The option will be removed** on [TBD removal date]. +- **With the option enabled**, the step log shows `WARNING: legacy_shell_expansion is enabled` (`helm`) or `WARNING: LEGACY_SHELL_EXPANSION is enabled` (`codefresh-run`), followed by the names of the arguments that still use shell syntax. That's the list of what needs to change. + +## How to update the pipeline and disable legacy mode + +1. Enable the temporary option (above) and run the pipeline once. +2. In the step log, find the warning. It lists the arguments to update, for example `CUSTOM_image_tag, CMD_PS` (`helm`) or `BRANCH VARIABLE` (`codefresh-run`). If it says that no arguments use shell syntax, skip to step 4. +3. Update each listed argument: + + | Before | After | + |---|---| + | `image_tag=$MY_TAG` | `image_tag=${{MY_TAG}}` | + | `VARIABLE: [IMAGE=$IMAGE_NAME]` | `VARIABLE: [IMAGE=${{IMAGE_NAME}}]` | + | `--set version=$(cat VERSION)` in `cmd_ps` | Add `cf_export VERSION=$(cat VERSION)` to an earlier freestyle step, then use `--set version=${{VERSION}}` | + | `--values values-*.yaml` in `cmd_ps` | List the files in `custom_value_files` | + | `kube_context: $MY_CONTEXT` | `kube_context: ${{MY_CONTEXT}}` | + | `BRANCH: main --no-cache` | `BRANCH: main` and `NO_CACHE: true` | + + Variables exported with `cf_export` in an earlier step are available as `${{VAR}}` in later steps. Pipeline and trigger variables are available as `${{VAR}}` too. + +4. Remove `legacy_shell_expansion` / `LEGACY_SHELL_EXPANSION` from the step and run the pipeline again. +5. Confirm that the warning is gone and the step works. + +## Escalate if + +- The pipeline still fails after the arguments are updated and legacy mode is removed. +- Legacy mode doesn't restore the previous behavior. +- The customer reports unexpected commands or network calls in the logs of these steps, or suspects their pipeline was abused. Escalate to the security team: [TBD]. + +## FAQ + +**Why can't we just keep the old behavior by default?** +Because it lets anyone who can push a branch to the customer's repository run commands in their pipeline. The temporary option exists only to give customers time to migrate. + +**Does this affect freestyle steps?** +No. `commands` in freestyle steps work as before. Only the arguments of the `codefresh-run` and `helm` steps changed. + +**The customer pinned an old `helm` step version. Why did it change?** +The step runs the `cfstep-helm` image, and the rebuilt image is published under the same tags. Every `helm` step version uses the new behavior. + +**The customer pinned `codefresh-run:1.6.0`. Are they affected?** +Not by the breaking change: the old version keeps its old behavior. Recommend upgrading to 1.7.0, because 1.6.0 and earlier contain the security issue. \ No newline at end of file From c5f8e4f4b5eceeca9bbc6caef3722b39def814b7 Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Thu, 29 Oct 2026 02:36:29 +0200 Subject: [PATCH 3/6] Revert "trigger commit" This reverts commit 49f4542971d34e18eaf76b38c64244be2e0da0d1. --- tests/cf2507-breaking-change-notice.md | 116 ----- tests/cf2507-interpolation-security.yml | 566 ------------------------ tests/cf2507-pr-descriptions.md | 199 --------- tests/cf2507-security-advisory.md | 133 ------ tests/cf2507-support-runbook.md | 134 ------ 5 files changed, 1148 deletions(-) delete mode 100644 tests/cf2507-breaking-change-notice.md delete mode 100644 tests/cf2507-interpolation-security.yml delete mode 100644 tests/cf2507-pr-descriptions.md delete mode 100644 tests/cf2507-security-advisory.md delete mode 100644 tests/cf2507-support-runbook.md diff --git a/tests/cf2507-breaking-change-notice.md b/tests/cf2507-breaking-change-notice.md deleted file mode 100644 index 961fc9c8d..000000000 --- a/tests/cf2507-breaking-change-notice.md +++ /dev/null @@ -1,116 +0,0 @@ -# Breaking change notice: argument handling in the codefresh-run and helm steps - - - -**Subject:** Action may be needed: the codefresh-run and helm pipeline steps change how they handle argument values on [TBD] - -Hi, - -On **[TBD release date]** we're releasing new versions of two official pipeline steps, **Run a Codefresh pipeline** (`codefresh-run` 1.7.0) and **Release a Helm chart** (`helm` 1.1.13), together with rebuilt `cfstep-helm` images. This is a security hardening change. - -Most pipelines keep working without any changes. This notice helps you check whether yours need an update, and explains how to update them. - -## What's changing - -Until now, these steps passed their argument values through a shell before using them. The shell evaluated some characters in the values. After the update, the steps use argument values exactly as you write them. - -| You write | Before the update | After the update | -|---|---|---| -| `${{MY_VAR}}` | The value of `MY_VAR` | No change: the value of `MY_VAR` | -| `$MY_VAR` or `${MY_VAR}` | The value of `MY_VAR` | The text `$MY_VAR` | -| `$(command)` or `` `command` `` | The output of the command | The text `$(command)` | -| `values-*.yaml` | The matching file names | The text `values-*.yaml` | -| `~/file` | The path in the home directory | The text `~/file` | -| `"main --no-cache"` in one argument | Several flags | One value: `main --no-cache` | - -Quotes and escaped commas in Helm values keep working the same way. For example, `profiles_arr="{one,two,three}"` and `profiles_str="one\,two\,three"` are passed to Helm as before. - -## Who's affected - -Your pipeline needs an update if a `codefresh-run` or `helm` step has an argument that uses `$VAR`, `$(...)`, backticks, `*`, or `~`. For the `helm` step, this includes `custom_values`, `custom_value_files`, `cmd_ps`, `kube_context`, `namespace`, and `release_name`. - -These pipelines get the new behavior: - -- Pipelines that use `type: codefresh-run` or `type: helm` without a version, from [TBD release date]. -- All `helm` step versions, because the rebuilt `cfstep-helm` images are published with the same tags. -- Pipelines that run a `cfstep-helm` image directly in a freestyle step. - -## How to check your pipelines - -1. Export the pipelines you store in Codefresh: - - ```bash - codefresh get pipelines --limit 1000 -o yaml > pipelines.yaml - ``` - -2. For pipelines stored in Git, check their `codefresh.yml` files in your repositories. -3. In each `codefresh-run` and `helm` step, look at the `arguments` for `$` (not followed by `{{`), `` ` ``, `*`, and `~`. For example: - - ```bash - grep -nE '\$[A-Za-z_(]|\$\{[^{]|`' pipelines.yaml - ``` - - This also finds `$VAR` in the `commands` of freestyle steps. Those don't change, so you can ignore them. - -You can also run a pipeline with the new step versions and look for a warning in the step log (see [Temporary option](#temporary-option) below). - -## How to update your pipelines - -| Before | After | -|---|---| -| `custom_values: [image_tag=$MY_TAG]` | `custom_values: [image_tag=${{MY_TAG}}]` | -| `VARIABLE: [IMAGE=$IMAGE_NAME]` | `VARIABLE: [IMAGE=${{IMAGE_NAME}}]` | -| `cmd_ps: --set version=$(cat VERSION)` | Read the file in a previous step: `cf_export VERSION=$(cat VERSION)`, then use `cmd_ps: --set version=${{VERSION}}` | -| `cmd_ps: --values values-*.yaml` | List the files in `custom_value_files` | -| `BRANCH: main --no-cache` | `BRANCH: main` and `NO_CACHE: true` | - -Variables you export with `cf_export` are available as `${{VAR}}` in all later steps. - -## Temporary option - -If you need more time to update a pipeline, you can get the previous behavior back for that step: - -| Step | Setting | -|---|---| -| `codefresh-run` 1.7.0 or later | `LEGACY_SHELL_EXPANSION: true` | -| `helm` 1.1.13 or later | `legacy_shell_expansion: true` | -| `cfstep-helm` image used directly | environment variable `LEGACY_SHELL_EXPANSION=true` | - -For example: - -```yaml -deploy: - type: helm:1.1.13 - arguments: - chart_name: my-chart - release_name: my-release - kube_context: my-cluster - custom_values: - - image_tag=$MY_TAG - legacy_shell_expansion: true # Temporary: remove after replacing $MY_TAG with ${{MY_TAG}} -``` - -When the option is enabled, the step log shows a warning with the names of the arguments that still need an update. If the warning says that no arguments use shell syntax, you can remove the option right away. - -This option is for migration only. It's less secure, and we'll remove it on **[TBD removal date]**. Please update your pipelines before then. - -Older `helm` step versions don't accept the new argument. To use it, upgrade the step to `helm` 1.1.13. - -## Timeline - -| Date | What happens | -|---|---| -| [TBD] | This notice | -| [TBD] | New step versions and rebuilt `cfstep-helm` images released | -| [TBD] | The temporary option is removed | - -## Questions - -If you have questions or need help updating your pipelines, contact [Codefresh support](https://support.codefresh.io). - -Happy deployments! \ No newline at end of file diff --git a/tests/cf2507-interpolation-security.yml b/tests/cf2507-interpolation-security.yml deleted file mode 100644 index fe0d2a51c..000000000 --- a/tests/cf2507-interpolation-security.yml +++ /dev/null @@ -1,566 +0,0 @@ -# CF-2507: Remote Code Execution via Codefresh Steps - verification pipeline -# -# Verifies the fixes in: -# - steps: graduated/codefresh-run (1.7.0), graduated/helm (1.1.13) -# - cfstep-helm: CF-2507-fix-interpolation-security (alpine) / CF-2507-fix-interpolation-security-debian (debian) images -# -# Every malicious payload tries to create its own marker file in /codefresh/volume/cf2507-markers. -# The shared volume is visible to all steps of the build, so a marker means that the payload was executed. -# Benign values check that existing pipelines keep working (quotes, spaces, colons, helm list syntax). -# -# The pipeline calls itself through codefresh-run: the child build (CF2507_MODE=child) verifies that -# the variables arrived unchanged. Do not define CF2507_MODE as a pipeline variable. -# -# Setup -# 1. Build the cfstep-helm images from both CF-2507 branches and note the image tags. -# 2. Publish private copies of the step types from this branch (replace ACCOUNT with your account name): -# yq '.metadata.name = "ACCOUNT/codefresh-run-cf2507" | .metadata.isPublic = false' graduated/codefresh-run/step.yaml > /tmp/codefresh-run-cf2507.yaml -# yq '.metadata.name = "ACCOUNT/helm-cf2507" | .metadata.isPublic = false' graduated/helm/step.yaml > /tmp/helm-cf2507.yaml -# codefresh create step-type -f /tmp/codefresh-run-cf2507.yaml -# codefresh create step-type -f /tmp/helm-cf2507.yaml -# 3. Replace __ACCOUNT__ in this file with your account name and create a pipeline with this YAML (no Git trigger is needed). -# 4. Pipeline variables: -# CF2507_HELM_VERSION (required) tag of quay.io/codefreshplugins/cfstep-helm to test, e.g. 3.14.0-- -# CF2507_HELM_DEBIAN_VERSION (required) tag of quay.io/codefreshplugins/cfstep-helm-debian to test -# CF2507_KUBE_CONTEXT (optional) name of a connected cluster; enables real `helm upgrade --install --dry-run` checks. -# Nothing is installed: the release is rendered with --dry-run only. -# 5. Run the pipeline. Expected result: all steps green. -# -# Negative check (proves the tests catch the vulnerability): switch the step types to the released vulnerable -# versions (codefresh-run:1.6.0, helm:1.1.12) and the helm image tags to the current released images. -# The "verify_*" steps must then fail and list the created markers. -# -# Manual check: in the log of run_child_with_values the "Executing command" line must show "$CFRUN_SECRET_0" -# and must not show the secret value. -# -# Legacy mode: the *_legacy steps enable the temporary legacy_shell_expansion option and check that it restores the -# previous behaviour for benign values ($VAR and $(...) are evaluated) and prints the migration warning. -# The default steps check that the option is disabled by default. - -version: '1.0' - -indicators: - - parentOnly: &parentOnly - condition: - all: - parentMode: 'includes("${{CF2507_MODE}}", "{{CF2507_MODE}}") == true' - - childOnly: &childOnly - condition: - all: - childMode: '"${{CF2507_MODE}}" == "child"' - - childLegacyOnly: &childLegacyOnly - condition: - all: - childMode: '"${{CF2507_MODE}}" == "child-legacy"' - - clusterOnly: &clusterOnly - condition: - all: - parentMode: 'includes("${{CF2507_MODE}}", "{{CF2507_MODE}}") == true' - hasCluster: 'includes("${{CF2507_KUBE_CONTEXT}}", "{{CF2507_KUBE_CONTEXT}}") == false' - - # Values passed to cfstep-helm in DRY_RUN mode. With DRY_RUN every helm/kubectl command is printed with echo, - # so the output shows exactly which arguments Helm would receive. - - environment: &helmDryRunEnv - - DRY_RUN=true - - ACTION=install - - SKIP_CF_STABLE_HELM_REPO=true - - CHART_REF=./chart - - KUBE_CONTEXT=$(touch /codefresh/volume/cf2507-markers/helm-kube-context) - - RELEASE_NAME=r;touch /codefresh/volume/cf2507-markers/helm-release - - NAMESPACE=`touch /codefresh/volume/cf2507-markers/helm-namespace` - - CHART_VERSION=1.0.0|touch /codefresh/volume/cf2507-markers/helm-chart-version - - CHART_REPO_URL=https://charts.example.com/$(touch /codefresh/volume/cf2507-markers/helm-repo-url) - - TIMEOUT=5m && touch /codefresh/volume/cf2507-markers/helm-timeout - - SET_FILE=cfg=$(touch /codefresh/volume/cf2507-markers/helm-set-file) - - 'CMD_PS=--atomic --description "$(touch /codefresh/volume/cf2507-markers/helm-cmd-ps)" ; touch /codefresh/volume/cf2507-markers/helm-cmd-ps-2' - - CUSTOM_image_tag=$(touch /codefresh/volume/cf2507-markers/helm-set) - - "VALUESTRING_image_name=x' && touch /codefresh/volume/cf2507-markers/helm-set-string && echo '" - - CUSTOMFILE_0=values.yaml;touch /codefresh/volume/cf2507-markers/helm-values-file - - CF_CTX_cf2507_URL=https://repo.example.com/$(touch /codefresh/volume/cf2507-markers/helm-ctx-repo) - - 'COMMIT_MESSAGE=x/e;touch /codefresh/volume/cf2507-markers/helm-commit-sed $(touch /codefresh/volume/cf2507-markers/helm-commit)' - # Backward compatibility: quote and backslash removal must keep working as before - - 'CUSTOM_pat_arr="{one,two,three}"' - - 'CUSTOM_STR__WITH__COMAS="one\,two\,three"' - - VALUESTRING_spaced=a b - - - commands: &helmDryRunCommands - - | - cd /codefresh/volume/cf2507 - printf 'Release notes\ncommit_message:previous\n' > chart/templates/NOTES.txt - rc=0 - bash /opt/bin/release_chart > helm-out.txt 2>&1 || rc=$? - cat helm-out.txt - if [ "$rc" != 0 ]; then echo "FAIL: release_chart exited with $rc"; exit 1; fi - if grep -q 'legacy_shell_expansion is enabled' helm-out.txt; then echo "FAIL: legacy mode must be disabled by default"; exit 1; fi - sh ./assert-helm-dry-run.sh helm-out.txt - - # Real Helm against a cluster, without installing anything (--dry-run) - - environment: &helmClusterEnv - - ACTION=install - - KUBE_CONTEXT=${{CF2507_KUBE_CONTEXT}} - - KUBECONFIG=${{CF_KUBECONFIG_PATH}} - - SKIP_CF_STABLE_HELM_REPO=true - - CHART_REF=./chart - - RELEASE_NAME=cf2507-test - - NAMESPACE=default - - CMD_PS=--dry-run --output json --description "cf2507 test" - - CUSTOM_image_tag=$(touch /codefresh/volume/cf2507-markers/helm-cluster-set) - - 'CUSTOM_pat_arr="{one,two,three}"' - - 'CUSTOM_STR__WITH__COMAS="one\,two\,three"' - - VALUESTRING_spaced=a b - - VALUESTRING_version=1.10 - - - commands: &helmClusterCommands - - | - cd /codefresh/volume/cf2507 - rc=0 - bash /opt/bin/release_chart > helm-cluster-out.txt 2>&1 || rc=$? - cat helm-cluster-out.txt - if [ "$rc" != 0 ]; then echo "FAIL: release_chart exited with $rc"; exit 1; fi - python3 ./verify-helm-cluster.py helm-cluster-out.txt - - # Legacy mode with benign values: the shell must evaluate $VAR and $(...) like before the fix - - environment: &helmLegacyEnv - - DRY_RUN=true - - ACTION=install - - LEGACY_SHELL_EXPANSION=true - - SKIP_CF_STABLE_HELM_REPO=true - - CHART_REF=./chart - - KUBE_CONTEXT=local - - RELEASE_NAME=cf2507-legacy - - MY_TAG=1.2.3 - - CUSTOM_image_tag=$MY_TAG - - CMD_PS=--description $(echo legacy-ok) - - - commands: &helmLegacyCommands - - | - cd /codefresh/volume/cf2507 - rc=0 - bash /opt/bin/release_chart > helm-legacy-out.txt 2>&1 || rc=$? - cat helm-legacy-out.txt - if [ "$rc" != 0 ]; then echo "FAIL: release_chart exited with $rc"; exit 1; fi - grep -qxF 'helm upgrade cf2507-legacy ./chart --install --reset-values --set image.tag=1.2.3 --description legacy-ok' helm-legacy-out.txt || { echo "FAIL: legacy mode didn't evaluate the values"; exit 1; } - grep -qF 'WARNING: legacy_shell_expansion is enabled' helm-legacy-out.txt || { echo "FAIL: legacy mode warning is missing"; exit 1; } - grep -qF 'behave differently once legacy_shell_expansion is disabled: CMD_PS, CUSTOM_image_tag' helm-legacy-out.txt || { echo "FAIL: legacy mode warning doesn't list the values"; exit 1; } - echo "OK: legacy mode evaluates values like before and prints the warning" - - - commands: &verifyMarkersCommands - - | - if [ -n "$(ls -A /codefresh/volume/cf2507-markers 2>/dev/null)" ]; then - echo "FAIL: injected commands were executed, markers:" - ls -1 /codefresh/volume/cf2507-markers - exit 1 - fi - echo "OK: no injected command was executed" - -stages: - - prepare - - codefresh-run - - helm-image - - helm-step - - helm-cluster - - child - -steps: - - # ---------------------------------------------------------------- prepare - prepare: - title: Prepare test chart and helper scripts - stage: prepare - image: alpine:3.20 - when: *parentOnly - commands: - - | - rm -rf /codefresh/volume/cf2507 /codefresh/volume/cf2507-markers - mkdir -p /codefresh/volume/cf2507/chart/templates /codefresh/volume/cf2507-markers - cd /codefresh/volume/cf2507 - - cat > chart/Chart.yaml <<'EOF' - apiVersion: v2 - name: cf2507 - version: 1.0.0 - EOF - - cat > chart/values.yaml <<'EOF' - {} - EOF - - cat > chart/templates/configmap.yaml <<'EOF' - apiVersion: v1 - kind: ConfigMap - metadata: - name: {{ .Release.Name }} - data: - values.json: {{ toJson .Values | quote }} - EOF - - # Arguments Helm must receive in DRY_RUN mode (printed by echo, one command per line) - cat > expected-helm-dry-run.txt <<'EOF' - kubectl config use-context $(touch /codefresh/volume/cf2507-markers/helm-kube-context) - helm repo add cf2507 https://repo.example.com/$(touch /codefresh/volume/cf2507-markers/helm-ctx-repo)/ - helm upgrade r;touch /codefresh/volume/cf2507-markers/helm-release ./chart --install --reset-values --repo https://charts.example.com/$(touch /codefresh/volume/cf2507-markers/helm-repo-url)/ --version 1.0.0|touch /codefresh/volume/cf2507-markers/helm-chart-version --namespace `touch /codefresh/volume/cf2507-markers/helm-namespace` --values values.yaml;touch /codefresh/volume/cf2507-markers/helm-values-file --set STR_WITH_COMAS=one\,two\,three --set image.tag=$(touch /codefresh/volume/cf2507-markers/helm-set) --set pat.arr={one,two,three} --set-string image.name=x && touch /codefresh/volume/cf2507-markers/helm-set-string && echo --set-string spaced=a b --timeout 5m && touch /codefresh/volume/cf2507-markers/helm-timeout --atomic --description $(touch /codefresh/volume/cf2507-markers/helm-cmd-ps) ; touch /codefresh/volume/cf2507-markers/helm-cmd-ps-2 --set-file cfg=$(touch /codefresh/volume/cf2507-markers/helm-set-file) - EOF - - cat > expected-notes.txt <<'EOF' - Release notes - commit_message:x/e;touch /codefresh/volume/cf2507-markers/helm-commit-sed $(touch /codefresh/volume/cf2507-markers/helm-commit) - EOF - - cat > assert-helm-dry-run.sh <<'EOF' - failed=0 - while IFS= read -r line; do - if grep -qxF -- "$line" "$1"; then - echo "OK: $line" - else - echo "FAIL: expected line not found: $line" - failed=1 - fi - done < /codefresh/volume/cf2507/expected-helm-dry-run.txt - if cmp -s /codefresh/volume/cf2507/expected-notes.txt /codefresh/volume/cf2507/chart/templates/NOTES.txt; then - echo "OK: commit message written literally to NOTES.txt" - else - echo "FAIL: unexpected NOTES.txt content:" - cat /codefresh/volume/cf2507/chart/templates/NOTES.txt - failed=1 - fi - exit $failed - EOF - - cat > dump-env.py <<'EOF' - import json, os, sys - with open(sys.argv[1], 'w') as f: - json.dump(dict(os.environ), f, indent=2, sort_keys=True) - EOF - - cat > verify-helm-step-env.py <<'EOF' - # Checks the environment the helm step template passed to the container - # Known issue (not related to CF-2507): in the debian image /bin/sh is dash, which drops environment - # variables with dots in the name (CUSTOM_pat.arr), so custom_values never reach the step there. - # With --allow-missing-dotted such variables are reported as SKIP when missing (they must still match when present). - import json, sys - env = json.load(open(sys.argv[1])) - allow_missing_dotted = '--allow-missing-dotted' in sys.argv[2:] - expected = { - 'NAMESPACE': "ns'\nprivileged: true", - 'RELEASE_NAME': 'r";$(touch /codefresh/volume/cf2507-markers/helm-step-release)', - 'CHART_VERSION': "1.0.0'\n - INJECTED_CHART_VERSION=1", - 'CUSTOM_pat.arr': '"{one,two,three}"', - 'CUSTOM_image.tag': "a'\n - INJECTED_CUSTOM=1", - 'CUSTOMFILE_0': "f'\n - INJECTED_FILE=1", - 'CF_CTX_cf2507_URL': "https://repo.example.com/x'\n - INJECTED_REPO=1", - 'SKIP_CF_STABLE_HELM_REPO': 'true', - 'ACTION': 'auth', - 'LEGACY_SHELL_EXPANSION': 'false', - } - failed = False - for key, value in expected.items(): - if env.get(key) == value: - print('OK: %s' % key) - elif allow_missing_dotted and '.' in key and key not in env: - print('SKIP: %s is missing (known dash issue, see above)' % key) - else: - print('FAIL: %s: got %r, expected %r' % (key, env.get(key), value)) - failed = True - injected = sorted(k for k in env if k.startswith('INJECTED')) - if injected: - print('FAIL: YAML injection created environment variables: %s' % injected) - failed = True - sys.exit(1 if failed else 0) - EOF - - cat > verify-helm-cluster.py <<'EOF' - # Parses `helm upgrade --output json` and checks the values Helm received - import json, sys - release = None - for line in open(sys.argv[1]): - if line.startswith('{') and '"manifest"' in line: - release = json.loads(line) - if release is None: - print('FAIL: helm release JSON not found in the output') - sys.exit(1) - expected = { - 'STR_WITH_COMAS': 'one,two,three', - 'image': {'tag': '$(touch /codefresh/volume/cf2507-markers/helm-cluster-set)'}, - 'pat': {'arr': ['one', 'two', 'three']}, - 'spaced': 'a b', - 'version': '1.10', - } - if release.get('config') != expected: - print('FAIL: unexpected values: %s' % json.dumps(release.get('config'), sort_keys=True)) - print('expected: %s' % json.dumps(expected, sort_keys=True)) - sys.exit(1) - if release.get('info', {}).get('description') != 'cf2507 test': - print('FAIL: unexpected description: %r' % release.get('info', {}).get('description')) - sys.exit(1) - print('OK: Helm received the expected values') - EOF - - ls -la /codefresh/volume/cf2507 - - # ---------------------------------------------------------------- codefresh-run - run_child_with_values: - title: codefresh-run - values reach the child pipeline literally - stage: codefresh-run - type: __ACCOUNT__/codefresh-run-cf2507 - when: *parentOnly - arguments: - PIPELINE_ID: '${{CF_PIPELINE_NAME}}' - DETACH: false - FOLLOW_LOGS: true - TIMEOUT_MINS: '15' - VARIABLE: - - 'CF2507_MODE=child' - - 'V_SPACES=value with spaces' - - 'V_COLON=key: value' - - 'V_QUOTES=it''s "quoted"' - - 'V_DOLLAR=$HOME' - - 'V_SUBST=$(touch /codefresh/volume/cf2507-markers/run-variable)' - - 'V_BACKTICK=`touch /codefresh/volume/cf2507-markers/run-backtick`' - - 'V_SEMI=x;touch /codefresh/volume/cf2507-markers/run-semicolon' - SECRETS: - - 'S_TOKEN=s3cr3t$(touch /codefresh/volume/cf2507-markers/run-secret)' - - run_injection_arguments: - title: codefresh-run - payloads in every string argument - stage: codefresh-run - type: __ACCOUNT__/codefresh-run-cf2507 - when: *parentOnly - # The CLI call itself is expected to fail (the trigger, branch and runtime don't exist) - fail_fast: false - arguments: - PIPELINE_ID: '${{CF_PIPELINE_NAME}}" ; touch /codefresh/volume/cf2507-markers/run-pipeline-id ; echo "' - TRIGGER_ID: 'trigger;touch /codefresh/volume/cf2507-markers/run-trigger' - BRANCH: '$(touch${IFS}/codefresh/volume/cf2507-markers/run-branch)' - SHA: '`touch /codefresh/volume/cf2507-markers/run-sha`' - RUNTIME_NAME: 'runtime && touch /codefresh/volume/cf2507-markers/run-runtime' - PACK_NAME: 'small|touch /codefresh/volume/cf2507-markers/run-pack' - VARIABLE_FILE: 'vars.yml;touch /codefresh/volume/cf2507-markers/run-var-file' - YAML: 'codefresh.yml;touch /codefresh/volume/cf2507-markers/run-yaml' - CONTEXT: - - 'ctx;touch /codefresh/volume/cf2507-markers/run-context' - ONLY: - - 'step;touch /codefresh/volume/cf2507-markers/run-only' - SKIP: - - 'step;touch /codefresh/volume/cf2507-markers/run-skip' - DETACH: true - - run_injection_timeout: - title: codefresh-run - payload in TIMEOUT_MINS - stage: codefresh-run - type: __ACCOUNT__/codefresh-run-cf2507 - when: *parentOnly - # `codefresh wait` is expected to reject the invalid timeout - fail_fast: false - arguments: - PIPELINE_ID: '${{CF_PIPELINE_NAME}}' - FOLLOW_LOGS: false - TIMEOUT_MINS: '10;touch /codefresh/volume/cf2507-markers/run-timeout' - VARIABLE: - - 'CF2507_MODE=noop' - - run_child_legacy: - title: codefresh-run - LEGACY_SHELL_EXPANSION evaluates values like before - stage: codefresh-run - type: __ACCOUNT__/codefresh-run-cf2507 - when: *parentOnly - arguments: - PIPELINE_ID: '${{CF_PIPELINE_NAME}}' - LEGACY_SHELL_EXPANSION: true - DETACH: false - FOLLOW_LOGS: true - TIMEOUT_MINS: '15' - VARIABLE: - - 'CF2507_MODE=child-legacy' - - 'V_LEGACY=$(echo legacy-ok)' - - verify_codefresh_run: - title: codefresh-run - no injected command was executed - stage: codefresh-run - image: alpine:3.20 - when: *parentOnly - commands: *verifyMarkersCommands - - # ---------------------------------------------------------------- cfstep-helm image - helm_image_alpine: - title: cfstep-helm image - generated script (alpine) - stage: helm-image - image: quay.io/codefreshplugins/cfstep-helm:${{CF2507_HELM_VERSION}} - when: *parentOnly - environment: *helmDryRunEnv - commands: *helmDryRunCommands - - verify_helm_image_alpine: - title: cfstep-helm image - no injected command was executed (alpine) - stage: helm-image - image: alpine:3.20 - when: *parentOnly - commands: *verifyMarkersCommands - - helm_image_debian: - title: cfstep-helm image - generated script (debian) - stage: helm-image - image: quay.io/codefreshplugins/cfstep-helm-debian:${{CF2507_HELM_DEBIAN_VERSION}} - when: *parentOnly - environment: *helmDryRunEnv - commands: *helmDryRunCommands - - verify_helm_image_debian: - title: cfstep-helm image - no injected command was executed (debian) - stage: helm-image - image: alpine:3.20 - when: *parentOnly - commands: *verifyMarkersCommands - - helm_image_legacy_alpine: - title: cfstep-helm image - legacy_shell_expansion evaluates values like before (alpine) - stage: helm-image - image: quay.io/codefreshplugins/cfstep-helm:${{CF2507_HELM_VERSION}} - when: *parentOnly - environment: *helmLegacyEnv - commands: *helmLegacyCommands - - helm_image_legacy_debian: - title: cfstep-helm image - legacy_shell_expansion evaluates values like before (debian) - stage: helm-image - image: quay.io/codefreshplugins/cfstep-helm-debian:${{CF2507_HELM_DEBIAN_VERSION}} - when: *parentOnly - environment: *helmLegacyEnv - commands: *helmLegacyCommands - - # ---------------------------------------------------------------- helm step template - # The auth action without kube_context doesn't need a cluster; the commands dump the container environment - helm_step_alpine: - title: helm step - argument values stay inside YAML strings (alpine) - stage: helm-step - type: __ACCOUNT__/helm-cf2507 - when: *parentOnly - arguments: - action: auth - helm_version: '${{CF2507_HELM_VERSION}}' - skip_cf_stable_helm_repo: true - namespace: "ns'\nprivileged: true" - release_name: 'r";$(touch /codefresh/volume/cf2507-markers/helm-step-release)' - chart_version: "1.0.0'\n - INJECTED_CHART_VERSION=1" - custom_values: - - 'pat.arr="{one,two,three}"' - - "image.tag=a'\n - INJECTED_CUSTOM=1" - custom_value_files: - - "f'\n - INJECTED_FILE=1" - repos: - - name: cf2507 - url: "https://repo.example.com/x'\n - INJECTED_REPO=1" - commands: - - python3 /codefresh/volume/cf2507/dump-env.py /codefresh/volume/cf2507/helm-step-alpine.json - - verify_helm_step_alpine: - title: helm step - verify container environment (alpine) - stage: helm-step - image: quay.io/codefreshplugins/cfstep-helm:${{CF2507_HELM_VERSION}} - when: *parentOnly - commands: - - python3 /codefresh/volume/cf2507/verify-helm-step-env.py /codefresh/volume/cf2507/helm-step-alpine.json - - helm_step_debian: - title: helm step - argument values stay inside YAML strings (debian) - stage: helm-step - type: __ACCOUNT__/helm-cf2507 - when: *parentOnly - arguments: - action: auth - use_debian_image: true - helm_version: '${{CF2507_HELM_DEBIAN_VERSION}}' - skip_cf_stable_helm_repo: true - namespace: "ns'\nprivileged: true" - release_name: 'r";$(touch /codefresh/volume/cf2507-markers/helm-step-release)' - chart_version: "1.0.0'\n - INJECTED_CHART_VERSION=1" - custom_values: - - 'pat.arr="{one,two,three}"' - - "image.tag=a'\n - INJECTED_CUSTOM=1" - custom_value_files: - - "f'\n - INJECTED_FILE=1" - repos: - - name: cf2507 - url: "https://repo.example.com/x'\n - INJECTED_REPO=1" - commands: - - python3 /codefresh/volume/cf2507/dump-env.py /codefresh/volume/cf2507/helm-step-debian.json - - verify_helm_step_debian: - title: helm step - verify container environment (debian) - stage: helm-step - image: quay.io/codefreshplugins/cfstep-helm:${{CF2507_HELM_VERSION}} - when: *parentOnly - commands: - - python3 /codefresh/volume/cf2507/verify-helm-step-env.py /codefresh/volume/cf2507/helm-step-debian.json --allow-missing-dotted - - verify_helm_step_markers: - title: helm step - no injected command was executed - stage: helm-step - image: alpine:3.20 - when: *parentOnly - commands: *verifyMarkersCommands - - # ---------------------------------------------------------------- real Helm (optional) - helm_cluster_alpine: - title: Real Helm receives the expected values (alpine) - stage: helm-cluster - image: quay.io/codefreshplugins/cfstep-helm:${{CF2507_HELM_VERSION}} - when: *clusterOnly - environment: *helmClusterEnv - commands: *helmClusterCommands - - helm_cluster_debian: - title: Real Helm receives the expected values (debian) - stage: helm-cluster - image: quay.io/codefreshplugins/cfstep-helm-debian:${{CF2507_HELM_DEBIAN_VERSION}} - when: *clusterOnly - environment: *helmClusterEnv - commands: *helmClusterCommands - - verify_helm_cluster: - title: Real Helm - no injected command was executed - stage: helm-cluster - image: alpine:3.20 - when: *clusterOnly - commands: *verifyMarkersCommands - - # ---------------------------------------------------------------- child mode (started by run_child_with_values) - child_verify_variables: - title: Child - variables from codefresh-run arrived unchanged - stage: child - image: alpine:3.20 - when: *childOnly - environment: - - GOT_SPACES=${{V_SPACES}} - - GOT_COLON=${{V_COLON}} - - GOT_QUOTES=${{V_QUOTES}} - - GOT_DOLLAR=${{V_DOLLAR}} - - GOT_SUBST=${{V_SUBST}} - - GOT_BACKTICK=${{V_BACKTICK}} - - GOT_SEMI=${{V_SEMI}} - - GOT_SECRET=${{S_TOKEN}} - commands: - - | - failed=0 - check() { - if [ "$1" = "$2" ]; then echo "OK: $3"; else echo "FAIL: $3: got [$1], expected [$2]"; failed=1; fi - } - check "$GOT_SPACES" 'value with spaces' V_SPACES - check "$GOT_COLON" 'key: value' V_COLON - check "$GOT_QUOTES" 'it'\''s "quoted"' V_QUOTES - check "$GOT_DOLLAR" '$HOME' V_DOLLAR - check "$GOT_SUBST" '$(touch /codefresh/volume/cf2507-markers/run-variable)' V_SUBST - check "$GOT_BACKTICK" '`touch /codefresh/volume/cf2507-markers/run-backtick`' V_BACKTICK - check "$GOT_SEMI" 'x;touch /codefresh/volume/cf2507-markers/run-semicolon' V_SEMI - check "$GOT_SECRET" 's3cr3t$(touch /codefresh/volume/cf2507-markers/run-secret)' S_TOKEN - exit $failed - - child_verify_legacy: - title: Child - LEGACY_SHELL_EXPANSION evaluated the value in the parent - stage: child - image: alpine:3.20 - when: *childLegacyOnly - environment: - - GOT_LEGACY=${{V_LEGACY}} - commands: - - | - if [ "$GOT_LEGACY" = 'legacy-ok' ]; then echo "OK: V_LEGACY"; else echo "FAIL: V_LEGACY: got [$GOT_LEGACY], expected [legacy-ok]"; exit 1; fi \ No newline at end of file diff --git a/tests/cf2507-pr-descriptions.md b/tests/cf2507-pr-descriptions.md deleted file mode 100644 index ccafa1830..000000000 --- a/tests/cf2507-pr-descriptions.md +++ /dev/null @@ -1,199 +0,0 @@ -# CF-2507 PR descriptions - -Three PRs, one per branch. `steps` and `cfstep-helm` are public repositories, so the descriptions explain the -class of the issue and the fix, but intentionally contain no working exploit or details from the pentest report. - ---- - -## 1. codefresh-io/steps — `CF-2507-fix-interpolation-security` → `master` - -**Title:** fix(CF-2507): prevent shell injection in codefresh-run and helm steps - -### Summary - -The `codefresh-run` step template interpolated raw argument values into a shell command. Arguments are often -derived from Git data (the documented example uses `BRANCH: '${{CF_BRANCH}}'`), so a crafted value, for example -a branch name, could execute arbitrary commands in the pipeline build container. - -The `helm` step template rendered argument values inside single-quoted YAML strings, so a quote or a new line in a -value could add arbitrary keys to the generated step. - -### Changes - -**codefresh-run 1.6.0 → 1.7.0** -- All arguments are passed to the step containers as environment variables (`CFRUN_*`) and are referenced in the - command only as `"$CFRUN_*"`. The command line contains no user input anymore. -- Array arguments (`VARIABLE`, `SECRETS`, `CONTEXT`, `ONLY`, `SKIP`) are passed as indexed variables - (`CFRUN_VARIABLE_0`, ...). -- `TIMEOUT_MINS` in the second step goes through the environment as well. -- Environment entries and the `image` are rendered with `toJSON`, so values can't break out of the YAML string. -- Side effect: secret values are no longer printed in the "Executing command" log line. - -**helm 1.1.12 → 1.1.13** -- Environment entries and the `image` are rendered with `toJSON` instead of `'...'`. -- `cmd_ps` documentation updated (schema description and README). -- The escaping of the generated Helm script is fixed in the `cfstep-helm` image (see related PRs). - -**Temporary migration option (both steps)** -- `codefresh-run`: `LEGACY_SHELL_EXPANSION` (boolean, default `false`). When enabled, the arguments are interpolated - exactly as in 1.6.0, and the step prints a warning with the names (not the values) of the arguments that contain - shell syntax. -- `helm`: `legacy_shell_expansion` (boolean, default `false`), passed to the image as `LEGACY_SHELL_EXPANSION`. -- The option is unsafe with user-controlled values. It only gives users time to migrate pipelines that rely on shell - expansion, and will be removed in a later version. - -### Behavior changes - -Argument values are now passed to the CLI literally: -- `$VAR`, `$(...)`, backticks, globs and `~` in arguments are no longer expanded by the shell. - Use `${{VAR}}` instead of `$VAR`; it is substituted by the engine before the step runs, as before. -- Several flags can't be passed through a single argument anymore (e.g. `BRANCH: "main --no-cache"`). -- Values with spaces now work as a single argument (`VARIABLE: ["key=value with spaces"]` was split before). - -Pipelines that need time to migrate can set `LEGACY_SHELL_EXPANSION: true` (`codefresh-run`) or -`legacy_shell_expansion: true` (`helm`) to get the previous behavior back temporarily. - -Pipelines that pin `codefresh-run:1.6.0` or older are not affected by this PR and should be upgraded. Pinned `helm` -step versions get the new behavior from the rebuilt `cfstep-helm` images; they can't set the new argument, so they -need to upgrade to `helm:1.1.13` to use the migration option. - -### Testing - -- Rendered both templates with the production template engine (pikolo 0.15.16): - - benign arguments: the rendered `helm` step is identical to the previous version; `codefresh run` receives the - same arguments as before, except values with spaces, which are now passed correctly; - - malicious arguments: the previous templates produced broken YAML or executed the injected commands, the new - ones keep every value as a literal string. -- Executed the rendered `codefresh-run` commands in `quay.io/codefresh/cli:0.89.1` with a stubbed CLI the same way - the engine does (`/bin/sh -c`, `set -e`): no injected command was executed, all values reached the CLI unchanged. -- `LEGACY_SHELL_EXPANSION: true`: the CLI receives exactly the same arguments as with `codefresh-run` 1.6.0 - (including `$VAR` and `$(...)` values), and the warning lists the arguments with shell syntax. With the default - value the rendered steps are identical to the version without the option. -- `tests/cf2507-interpolation-security.yml`: verification pipeline covering all three PRs (not committed yet; - it hasn't been run in Codefresh yet). - -### Related PRs - -- codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security` (alpine images) -- codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security-debian` (debian images) - -🤖 Generated with [Claude Code](https://claude.com/claude-code) - ---- - -## 2. codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security` → `master` - -**Title:** fix(CF-2507): prevent shell injection in generated helm script - -### Summary - -`EntrypointScriptBuilder` generates `/tmp/run` by interpolating environment values (`KUBE_CONTEXT`, `RELEASE_NAME`, -`NAMESPACE`, `custom_values`, `CMD_PS`, repository URLs, ...) into shell commands without escaping. When a pipeline -passes Git-derived data to the helm step, a crafted value could execute arbitrary commands in the build container. -`COMMIT_MESSAGE` was additionally interpolated into a `sed` expression, which allowed command execution through -the `e` flag of GNU sed. - -### Changes - -- Every value interpolated into the generated script is quoted with `shlex.quote`. -- `--set` / `--set-string` values are parsed with `shlex.split` before quoting. This keeps the existing quote and - backslash removal (e.g. `pat.arr="{one,two,three}"`, `one\,two\,three`) without performing any expansion. - A value that is not a single shell word is passed literally. -- `CMD_PS` is split and re-quoted token by token: quotes are respected, but `$VAR`, `$(...)` and shell operators are - passed to Helm literally. Unbalanced quotes fail with `Failed to parse CMD_PS`. -- `COMMIT_MESSAGE` is passed to `awk` through the environment instead of being interpolated into shell/sed code. - Multi-line commit messages now work (they broke `sed` before). -- `$PACKAGE`, `$HELMREPO_USERNAME:$HELMREPO_PASSWORD` and `$GOOGLE_APPLICATION_CREDENTIALS_JSON` are quoted. -- Temporary migration option `LEGACY_SHELL_EXPANSION` (`legacy_shell_expansion` step argument), disabled by - default. An unset variable, including all older `helm` step versions, uses the safe behavior. When enabled, values - provided by the user are interpolated as before, and a warning lists the names (not the values) of the variables - that contain shell syntax. `COMMIT_MESSAGE` and internal variables stay escaped in both modes. -- Tests: new tests execute the generated script with stubbed `helm`/`kubectl` and assert that no injected command is - executed and that Helm receives the expected arguments. `assertEquals` replaced with `assertEqual` (removed in - Python 3.12, two tests were failing before this change). - -### Behavior changes - -- `$VAR`, `$(...)`, globs and `~` in values and in `CMD_PS` are no longer expanded, unless `LEGACY_SHELL_EXPANSION` - is enabled. -- Values with `;` or spaces in `--set-string`, and values like `it's`, now work (they broke the command before). -- The script printed in the log ("Running the following script") now contains quotes. Credential masking still works. - -The image tags stay the same (`cfstep-helm:`), so after the images are rebuilt the fix applies to all -versions of the `helm` step, including the ones pinned to older step versions. - -### Testing - -- `python3 -m unittest tests/EntrypointScriptBuilderTest.py`: 26/26 locally and inside - `quay.io/codefreshplugins/cfstep-helm:3.14.0` (Python 3.9, busybox awk). -- The new tests fail against `origin/master` (the injected commands are executed). -- With `LEGACY_SHELL_EXPANSION=true` Helm receives exactly the same arguments as with `origin/master`, including - `$VAR` values and globs. -- Compared the arguments Helm receives before and after the change for typical values: identical, except for the - cases that were broken before (listed above). -- Ran the image in `DRY_RUN` mode with malicious values for every input: no command was executed, Helm received - the values literally. -- Not run: robot acceptance tests (they need real `helm`/`kubectl` binaries). - -### Related PRs - -- codefresh-io/steps — `CF-2507-fix-interpolation-security` (helm 1.1.13, codefresh-run 1.7.0) -- codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security-debian` (same fix for the debian images) - -🤖 Generated with [Claude Code](https://claude.com/claude-code) - ---- - -## 3. codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security-debian` → `debian` - -**Title:** fix(CF-2507): prevent shell injection in generated helm script (debian) - -### Summary - -Port of the master fix to the `debian` branch, which the `cfstep-helm-debian` images (`use_debian_image: true`) are -built from. Without it the debian images stay vulnerable after the rebuild. - -`EntrypointScriptBuilder` generates `/tmp/run` by interpolating environment values into shell commands without -escaping, and `COMMIT_MESSAGE` was interpolated into a `sed` expression. When a pipeline passes Git-derived data to -the helm step, a crafted value could execute arbitrary commands in the build container. - -### Changes - -Same as the master PR: -- every value interpolated into the generated script is quoted with `shlex.quote`; -- `--set` / `--set-string` values keep the existing quote and backslash removal without expansions; -- `CMD_PS` is re-quoted token by token; -- `COMMIT_MESSAGE` is passed to `awk` through the environment; -- `$PACKAGE`, Helm repository credentials and `$GOOGLE_APPLICATION_CREDENTIALS_JSON` are quoted; -- the same temporary `LEGACY_SHELL_EXPANSION` migration option, disabled by default; -- the same tests as in the master PR. - -The `debian` branch has an older `EntrypointScriptBuilder` (no per-repository credentials, no -`use_repos_for_auth_action`). The conflict in the `helm repo add` block was resolved by keeping the debian logic and -adding the escaping only. No functionality from master was ported. - -### Behavior changes - -Same as the master PR: `$VAR`, `$(...)`, globs and `~` in values and in `CMD_PS` are no longer expanded, unless -`LEGACY_SHELL_EXPANSION` is enabled. - -### Testing - -- `python3 -m unittest tests/EntrypointScriptBuilderTest.py`: 26/26 locally and inside - `quay.io/codefreshplugins/cfstep-helm-debian:3.14.0` (Python 3.8, mawk). -- The new tests fail against `origin/debian` (the injected commands are executed). -- With `LEGACY_SHELL_EXPANSION=true` Helm receives exactly the same arguments as with `origin/debian`. -- Ran the image in `DRY_RUN` mode with malicious values for every input: no command was executed. - -### Known issue (not related to this PR) - -In the debian image `/bin/sh` is `dash`, and the engine runs step commands with `/bin/sh -c`. `dash` drops -environment variables with dots in the name, so `custom_values` (`CUSTOM_`) never reach the step when -`use_debian_image: true` is set. This happens before and after this change; it should be tracked separately. - -### Related PRs - -- codefresh-io/steps — `CF-2507-fix-interpolation-security` (helm 1.1.13, codefresh-run 1.7.0) -- codefresh-contrib/cfstep-helm — `CF-2507-fix-interpolation-security` (same fix for master / alpine images) - -🤖 Generated with [Claude Code](https://claude.com/claude-code) \ No newline at end of file diff --git a/tests/cf2507-security-advisory.md b/tests/cf2507-security-advisory.md deleted file mode 100644 index d54b066a0..000000000 --- a/tests/cf2507-security-advisory.md +++ /dev/null @@ -1,133 +0,0 @@ -# Security advisory: command injection in the codefresh-run and helm pipeline steps - - - -| | | -|---|---| -| **Advisory ID** | [TBD] | -| **CVE** | [TBD] | -| **Severity** | High | -| **Published** | [TBD] | -| **Affected products** | Codefresh pipelines (SaaS, hybrid, and on-premises) that use the `codefresh-run` or `helm` steps | -| **Internal reference** | CF-2507 | - -## Summary - -We found and fixed a command injection issue in two official Codefresh pipeline steps: **Run a Codefresh pipeline** (`codefresh-run`) and **Release a Helm chart** (`helm`). - -Both steps built shell commands from their arguments without escaping them. When a pipeline passes data that someone outside your team can control, such as a Git branch name or a commit message, to one of these arguments, that person can run their own commands in the pipeline build. They only need permission to push to a repository that triggers the pipeline. They don't need a Codefresh account. - -A successful attack runs with the permissions of the pipeline build. This includes access to the Codefresh API key of the user or service account that runs the pipeline, and to any secrets, registries, and clusters the build can reach. - -We aren't aware of this issue being exploited. We recommend that you upgrade as soon as possible. - -## Are you affected? - -You're affected if **all** of the following are true: - -1. A pipeline uses the `codefresh-run` step at version 1.6.0 or earlier, or the `helm` step at version 1.1.12 or earlier, or runs a `cfstep-helm` image directly as a freestyle step. -2. At least one step argument contains a value that someone outside your team can influence. Common examples: - - `${{CF_BRANCH}}`, `${{CF_COMMIT_MESSAGE}}`, `${{CF_PULL_REQUEST_TITLE}}`, or other `${{CF_PULL_REQUEST_*}}` variables - - variables derived from Git data, webhook payloads, or other external input -3. The pipeline runs automatically for that input, for example a Git trigger that runs on any branch (the default branch filter is `/.*/gi`). - -The documented `codefresh-run` example uses `BRANCH: '${{CF_BRANCH}}'`. If your pipelines follow this example and use a Git trigger, assume you're affected. - -| Step | Affected arguments | -|---|---| -| `codefresh-run` | `PIPELINE_ID`, `TRIGGER_ID`, `BRANCH`, `SHA`, `RUNTIME_NAME`, `PACK_NAME`, `VARIABLE`, `SECRETS`, `VARIABLE_FILE`, `CONTEXT`, `ONLY`, `SKIP`, `YAML`, and `TIMEOUT_MINS` | -| `helm` | All string arguments, including `kube_context`, `release_name`, `namespace`, `chart_name`, `chart_version`, `custom_values`, `custom_value_files`, `cmd_ps`, and repository URLs | - -## Fixed versions - -| Component | Fixed version | -|---|---| -| `codefresh-run` step | 1.7.0 | -| `helm` step | 1.1.13 | -| `quay.io/codefreshplugins/cfstep-helm` images | All tags rebuilt on [TBD] | -| `quay.io/codefreshplugins/cfstep-helm-debian` images | All tags rebuilt on [TBD] | - -The fixed steps pass argument values to the container as environment variables and never place them in a shell command. The `cfstep-helm` images escape every value they add to the generated Helm commands. - -## What you need to do - -### SaaS and hybrid runtimes - -1. **Pipelines that don't pin a step version** (`type: codefresh-run`, `type: helm`) use the fixed versions automatically. Check them for the [behavior changes](#behavior-changes) below. -2. **Pipelines that pin a step version** (for example `type: codefresh-run:1.6.0` or `type: helm:1.1.12`): update them to `codefresh-run:1.7.0` and `helm:1.1.13` or later, or remove the version to always use the latest one. -3. **Pipelines that run `cfstep-helm` images directly**: pull the image again. If you pin the image by digest, or copy it to your own registry, update the digest or mirror the rebuilt image. - -The rebuilt `cfstep-helm` images also protect older `helm` step versions from command injection. We still recommend upgrading to `helm` 1.1.13, because older versions of the step template can let a crafted value add unexpected settings to the generated step. - -### If a pipeline stops working after the upgrade - -If a pipeline relies on the shell evaluating argument values (see [Behavior changes](#behavior-changes)), you can temporarily get the previous behavior back while you update it: - -| Step | Setting | -|---|---| -| `codefresh-run` 1.7.0 or later | `LEGACY_SHELL_EXPANSION: true` | -| `helm` 1.1.13 or later | `legacy_shell_expansion: true` | -| `cfstep-helm` image used directly | environment variable `LEGACY_SHELL_EXPANSION=true` | - -When the option is enabled, the step log shows a warning with the names of the arguments that still use shell syntax. Update those arguments, then remove the option. - -> **Warning:** this option brings the vulnerability back for that step. Use it only for pipelines that don't receive values from outside your team, and only for as long as you need to migrate. We'll remove the option in a later version of the steps. - -Older `helm` step versions don't accept the new argument. To use it, upgrade the step to `helm` 1.1.13. - -### On-premises installations - -In addition to the steps above, make sure your installation uses the rebuilt `cfstep-helm` and `cfstep-helm-debian` images. If you mirror images to a private registry, mirror them again after [TBD]. - -### Check for signs of abuse - -If you were affected, review recent builds of pipelines that use these steps: - -- Look for builds triggered by branch names or commit messages that contain shell characters, such as `$(`, backticks, `;`, `|`, or `&&`. -- Look for unexpected network connections or commands in the logs of the `codefresh-run` and `helm` steps. - -If you find anything suspicious, rotate the Codefresh API keys of the users and service accounts that run those pipelines, and rotate any secrets the affected builds could access. - -## Behavior changes - -The fix changes how argument values are handled. Most pipelines keep working without changes. Check your pipelines if they rely on any of the following. Until you update them, you can use the [temporary option](#if-a-pipeline-stops-working-after-the-upgrade) to keep the previous behavior. - -- **Shell expansion in arguments.** `$VAR`, `$(...)`, backticks, wildcards (`*`), and `~` are now passed literally. Use Codefresh variables (`${{VAR}}`) instead. The engine replaces them before the step runs, the same way as before. Variables you export with `cf_export` are available as `${{VAR}}` in later steps. -- **Several flags in one `codefresh-run` argument.** For example, `BRANCH: "main --no-cache"` now passes the whole value as the branch name. Use the dedicated arguments, such as `NO_CACHE: true`, instead. -- **`cmd_ps` in the `helm` step.** Quotes still work as before (`--description "my release"`), but shell operators and expansions are passed to Helm literally. - -Some values that didn't work before now work correctly. These include `codefresh-run` variables with spaces, and `helm` values that contain `;`, spaces, or apostrophes. Quotes and escaped commas in `custom_values`, such as `pat.arr="{one,two,three}"` or `key="one\,two"`, keep working the same way. - -The `codefresh-run` step also no longer prints secret values in the "Executing command" line of the build log. - -## Workarounds - -If you can't upgrade right away: - -- Don't pass Git-derived values to the affected arguments. If you need a branch name, use `${{CF_BRANCH_TAG_NORMALIZED}}` instead of `${{CF_BRANCH}}`. It contains only characters that are valid in a Docker image tag, so it can't be used to inject commands. Don't rely on the `*_ESCAPED` variables for this: they aren't designed to make values safe for a shell. -- Restrict Git triggers to the branches you trust, instead of the default `/.*/gi` filter. -- Limit who can push to repositories that trigger pipelines with these steps. - -## Timeline - -| Date | Event | -|---|---| -| [TBD] | Issue reported during a security assessment | -| [TBD] | Fixed step versions released | -| [TBD] | All `cfstep-helm` images rebuilt | -| [TBD] | Advisory published | -| [TBD] | `LEGACY_SHELL_EXPANSION` / `legacy_shell_expansion` option removed | - -## Credit - -[TBD: confirm with the reporter whether and how they want to be credited.] - -## Questions - -If you have questions about this advisory, contact [Codefresh support](https://support.codefresh.io). \ No newline at end of file diff --git a/tests/cf2507-support-runbook.md b/tests/cf2507-support-runbook.md deleted file mode 100644 index 9a46e683a..000000000 --- a/tests/cf2507-support-runbook.md +++ /dev/null @@ -1,134 +0,0 @@ -# Support runbook: codefresh-run and helm steps no longer evaluate shell syntax in arguments - - - -| | | -|---|---| -| **Released** | [TBD] | -| **Affected steps** | `codefresh-run` 1.7.0 and later; `helm` (all versions) after the `cfstep-helm` images were rebuilt on [TBD]; `cfstep-helm` / `cfstep-helm-debian` images used directly | -| **Temporary option removed on** | [TBD] | -| **Escalation** | [TBD: team / channel] | - -## What changed - -As a security fix, the `codefresh-run` and `helm` steps now use argument values exactly as written. Before, the values went through a shell, which evaluated `$VAR`, `$(...)`, backticks, wildcards (`*`), and `~`. Pipelines that relied on this get the literal text now, for example `$MY_TAG` instead of `1.2.3`. - -`${{VAR}}` (Codefresh variables) works exactly as before. - -## How to identify the issue - -It's most likely this issue if **all** of the following are true: - -1. The failing step is `codefresh-run` or `helm`, or a freestyle step that uses a `cfstep-helm` image. -2. The pipeline worked before [TBD release date] and its YAML didn't change. -3. A step argument contains `$` that isn't part of `${{...}}`, a backtick, `*`, or `~`. - -Typical symptoms: - -| Step | What the customer sees | -|---|---| -| `helm` | Values contain the literal text, e.g. image `my-repo:$MY_TAG` and `InvalidImageName` in Kubernetes | -| `helm` | `no context exists with the name: "$MY_CONTEXT"` | -| `helm` | `open values-*.yaml: no such file or directory` | -| `helm` | Helm complains about unexpected arguments when `cmd_ps` contains `;`, `&&`, or `\|` | -| `helm` | `Failed to parse CMD_PS: No closing quotation` (new error, `cmd_ps` has an unclosed quote) | -| `codefresh-run` | The child pipeline receives `$VAR` as a variable value instead of the real value | -| `codefresh-run` | The child pipeline isn't found, or the wrong branch is used, when `PIPELINE_ID` or `BRANCH` contains `$VAR` | -| `codefresh-run` | `codefresh wait` rejects the timeout when `TIMEOUT_MINS` contains `$VAR` | - -To confirm, open the step log: - -- **`helm`**: in the "Running the following script" section, the values appear in single quotes, e.g. `--set 'image.tag=$MY_TAG'`. -- **`codefresh-run`**: the "Executing command" line shows `"$CFRUN_..."` placeholders. Compare them with the step `arguments` in the pipeline YAML. - -### Not this issue - -- **`helm` with `use_debian_image: true` ignores `custom_values` written with dots in the key** (e.g. `image.tag=1.2.3`, no `$` involved). This is a separate known issue, CF-2559. Fix: use the documented format with underscores (`image_tag=1.2.3`); the step converts `_` to `.`. -- The step failed before [TBD release date] as well, or the pipeline YAML changed recently. - -## What to tell the customer - -Short explanation you can use as is: - -> We recently released a security update for the `codefresh-run` and `helm` steps. The steps now use argument values exactly as they're written, instead of passing them through a shell. Your pipeline uses shell syntax (`$VAR`) in a step argument, so the step now receives the text `$VAR` instead of its value. -> -> You can fix this by replacing the shell variables with Codefresh variables (`${{VAR}}`). If you need more time, you can temporarily restore the previous behavior with the option below. - -Don't share details about the vulnerability beyond what's in the published advisory: [TBD advisory link]. - -## Workaround: temporary legacy mode - -The customer adds one setting to the affected step: - -| Step | Setting | -|---|---| -| `codefresh-run` 1.7.0 or later | `LEGACY_SHELL_EXPANSION: true` | -| `helm` 1.1.13 or later | `legacy_shell_expansion: true` | -| `cfstep-helm` image used directly | environment variable `LEGACY_SHELL_EXPANSION=true` | - -```yaml -deploy: - type: helm:1.1.13 - arguments: - # ...existing arguments... - legacy_shell_expansion: true # Temporary -``` - -```yaml -run_child: - type: codefresh-run - arguments: - # ...existing arguments... - LEGACY_SHELL_EXPANSION: true # Temporary -``` - -Things to tell the customer: - -- **Older `helm` step versions** (`helm:1.1.12` and earlier) don't accept the new argument and fail with a validation error. Upgrade the step to `helm:1.1.13` first. -- **The option is less secure**: it brings back the behavior that the security update fixed. Recommend it only as a short-term fix, and only for pipelines that don't receive values from outside the customer's team (for example, branch names from a Git trigger). -- **The option will be removed** on [TBD removal date]. -- **With the option enabled**, the step log shows `WARNING: legacy_shell_expansion is enabled` (`helm`) or `WARNING: LEGACY_SHELL_EXPANSION is enabled` (`codefresh-run`), followed by the names of the arguments that still use shell syntax. That's the list of what needs to change. - -## How to update the pipeline and disable legacy mode - -1. Enable the temporary option (above) and run the pipeline once. -2. In the step log, find the warning. It lists the arguments to update, for example `CUSTOM_image_tag, CMD_PS` (`helm`) or `BRANCH VARIABLE` (`codefresh-run`). If it says that no arguments use shell syntax, skip to step 4. -3. Update each listed argument: - - | Before | After | - |---|---| - | `image_tag=$MY_TAG` | `image_tag=${{MY_TAG}}` | - | `VARIABLE: [IMAGE=$IMAGE_NAME]` | `VARIABLE: [IMAGE=${{IMAGE_NAME}}]` | - | `--set version=$(cat VERSION)` in `cmd_ps` | Add `cf_export VERSION=$(cat VERSION)` to an earlier freestyle step, then use `--set version=${{VERSION}}` | - | `--values values-*.yaml` in `cmd_ps` | List the files in `custom_value_files` | - | `kube_context: $MY_CONTEXT` | `kube_context: ${{MY_CONTEXT}}` | - | `BRANCH: main --no-cache` | `BRANCH: main` and `NO_CACHE: true` | - - Variables exported with `cf_export` in an earlier step are available as `${{VAR}}` in later steps. Pipeline and trigger variables are available as `${{VAR}}` too. - -4. Remove `legacy_shell_expansion` / `LEGACY_SHELL_EXPANSION` from the step and run the pipeline again. -5. Confirm that the warning is gone and the step works. - -## Escalate if - -- The pipeline still fails after the arguments are updated and legacy mode is removed. -- Legacy mode doesn't restore the previous behavior. -- The customer reports unexpected commands or network calls in the logs of these steps, or suspects their pipeline was abused. Escalate to the security team: [TBD]. - -## FAQ - -**Why can't we just keep the old behavior by default?** -Because it lets anyone who can push a branch to the customer's repository run commands in their pipeline. The temporary option exists only to give customers time to migrate. - -**Does this affect freestyle steps?** -No. `commands` in freestyle steps work as before. Only the arguments of the `codefresh-run` and `helm` steps changed. - -**The customer pinned an old `helm` step version. Why did it change?** -The step runs the `cfstep-helm` image, and the rebuilt image is published under the same tags. Every `helm` step version uses the new behavior. - -**The customer pinned `codefresh-run:1.6.0`. Are they affected?** -Not by the breaking change: the old version keeps its old behavior. Recommend upgrading to 1.7.0, because 1.6.0 and earlier contain the security issue. \ No newline at end of file From 70b1c26cc09077fbc4ea92eeebe53dd1f7eccf29 Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Thu, 29 Oct 2026 03:06:58 +0200 Subject: [PATCH 4/6] trigger commit From 233387a009a29703baa6223cfba48e675fff9e38 Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Fri, 30 Oct 2026 14:25:57 +0200 Subject: [PATCH 5/6] fix(CF-2507): list the shell-evaluated arguments of codefresh-run and helm in spec.shellEvaluatedArguments Replace the x-shell-evaluated marks in the argument schemas with spec.shellEvaluatedArguments, which pipeline-manager validates. The argument schemas are the same as before. The planner passes Git variables in the listed arguments as environment variables, so the shell does not evaluate their values. --- graduated/codefresh-run/step.yaml | 39 ++++++++++++++-------------- graduated/helm/step.yaml | 43 ++++++++++++++++--------------- 2 files changed, 42 insertions(+), 40 deletions(-) diff --git a/graduated/codefresh-run/step.yaml b/graduated/codefresh-run/step.yaml index ad3aaa899..6eac14250 100644 --- a/graduated/codefresh-run/step.yaml +++ b/graduated/codefresh-run/step.yaml @@ -40,11 +40,6 @@ metadata: latest: true id: 5e0231f76e1ebe58d5d37a37 spec: - # "x-shell-evaluated" marks the arguments that stepsTemplate puts into the shell command, and where: - # "unquoted", or "double-quoted" when the template puts the argument inside double quotes. - # In these arguments the engine passes the Git variables, such as CF_BRANCH, to the shell as environment variables, - # so that the shell doesn't run commands from a branch name or a commit message. - # Update the marks when you change how stepsTemplate uses an argument (see ShellArguments.js in the engine). arguments: |- { "definitions": {}, @@ -63,25 +58,21 @@ spec: "default": "0.89.1" }, "PIPELINE_ID": { - "x-shell-evaluated": "double-quoted", "type": "string", "description": "Name or ID of pipeline to run. In the UI, select Pipelines. Click pipeline name, then select Settings, and get the Name and ID." }, "TRIGGER_ID": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Name or ID of trigger for specified pipeline. Mandatory when BRANCH or SHA are defined. In the UI, select Pipelines. Click pipeline name, then select Workflows > Triggers. Click Edit and get the Name. ", "default": "" }, "BRANCH": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Branch for which to run the pipeline. When defined, TRIGGER_ID must also be defined.", "examples": ["master"], "default": "" }, "SHA": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Git commit hash. When defined, TRIGGER_ID must also be defined.", "examples": ["c777d850ff67969cb4eb9e041d7edeb367705e12"], @@ -108,14 +99,12 @@ spec: "default": false }, "RUNTIME_NAME": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Runtime environment to run the pipeline.", "examples": ["my-runtime-name"], "default": "" }, "PACK_NAME": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Resources size (small, medium or large).", "default": "" @@ -131,7 +120,6 @@ spec: "default": true }, "CONTEXT": { - "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -141,7 +129,6 @@ spec: "default": [] }, "VARIABLE": { - "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -151,7 +138,6 @@ spec: "default": [] }, "SECRETS": { - "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -161,14 +147,12 @@ spec: "default": [] }, "VARIABLE_FILE": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Import build variables from a file.", "examples": ["/codefresh/volume/my-repo/var_file_example.yml"], "default": "" }, "TIMEOUT_MINS": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Timeout in minutes after which to consider pipeline build as failed.", "examples": ["33.3", "45"], @@ -180,7 +164,6 @@ spec: "default": false }, "ONLY": { - "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -190,7 +173,6 @@ spec: "default": [] }, "SKIP": { - "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -200,13 +182,32 @@ spec: "default": [] }, "YAML": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Path to pipeline's yaml to be used as override for this execution.", "default": "" } } } + # shellEvaluatedArguments lists the arguments that stepsTemplate puts into the shell command, and where: + # "unquoted", or "double-quoted" when the template puts the argument inside double quotes. + # In these arguments the engine passes the Git variables, such as CF_BRANCH, to the shell as environment variables, + # so that the shell doesn't run commands from a branch name or a commit message. + # Update the list when you change how stepsTemplate uses an argument (see ShellArguments in the planner). + shellEvaluatedArguments: + PIPELINE_ID: double-quoted + TRIGGER_ID: unquoted + BRANCH: unquoted + SHA: unquoted + RUNTIME_NAME: unquoted + PACK_NAME: unquoted + CONTEXT: unquoted + VARIABLE: unquoted + SECRETS: unquoted + VARIABLE_FILE: unquoted + TIMEOUT_MINS: unquoted + ONLY: unquoted + SKIP: unquoted + YAML: unquoted delimiters: left: '[[' right: ']]' diff --git a/graduated/helm/step.yaml b/graduated/helm/step.yaml index 39ab12d05..8f1c1aa30 100644 --- a/graduated/helm/step.yaml +++ b/graduated/helm/step.yaml @@ -73,12 +73,6 @@ metadata: commands: - helm list spec: - # "x-shell-evaluated" marks the arguments that end up in a shell command, and where: "unquoted", or "double-quoted" - # when the command has the argument inside double quotes. The entrypoint script of the image builds the command - # from the environment variables that stepsTemplate creates from the arguments. - # In these arguments the engine passes the Git variables, such as CF_BRANCH, to the shell as environment variables, - # so that the shell doesn't run commands from a branch name or a commit message. - # Update the marks when you change how an argument is used (see ShellArguments.js in the engine). arguments: |- { "definitions": {}, @@ -90,7 +84,6 @@ spec: ], "properties": { "kube_context": { - "x-shell-evaluated": "double-quoted", "type": "string", "description": "Kubernetes context to use (the name of the cluster as configured in Codefresh) (required)" }, @@ -107,12 +100,10 @@ spec: "description": "Add helm repositories from contexts for auth action" }, "chart_name": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Helm chart name to release (path to chart folder, or name of packaged chart)" }, "release_name": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Helm release name" }, @@ -121,23 +112,19 @@ spec: "description": "The helm operation mode is set by the ACTION variable, where the value is install/auth/push" }, "namespace": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Target Kubernetes namespace" }, "tiller_namespace": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Kubernetes namespace where tiller is at" }, "chart_version": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Application chart version to install" }, "app_version": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Version of the application. This field is informational, and has no impact on chart version calculations." }, @@ -160,12 +147,10 @@ spec: "description": "Version of cfstep-helm image" }, "chart_repo_url": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Helm chart repository URL (overriden by injected Helm repository context)" }, "custom_value_files": { - "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -173,7 +158,6 @@ spec: "description": "Values file to provide to Helm (as --values or -f). see usage information below" }, "custom_values": { - "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -181,12 +165,10 @@ spec: "description": "Variables to provide to Helm (as --set). see usage information below" }, "cmd_ps": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Variable to provide other Helm cli flags. For example: '--wait --timeout', etc" }, "set_file": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "Set values from respective files specified via the command line (can specify multiple or separate values with commas: key1=path1,key2=path2)" }, @@ -195,12 +177,10 @@ spec: "description": "Waits until all Pods are in a ready state, PVCs are bound, Deployments have minimum Pods in ready state and Services have an IP address before marking the release as successful" }, "timeout": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "A value in seconds to wait for Kubernetes commands to complete" }, "commands": { - "x-shell-evaluated": "unquoted", "type": "array", "items": { "type": "string" @@ -216,7 +196,6 @@ spec: "description": "Add credentials to helm command as arguments (username, password). Otherwise its will be added to url http(s)://username:password@url. Should be enabled for JFrog Artifactory helm repository." }, "chart_subdir": { - "x-shell-evaluated": "unquoted", "type": "string", "description": "optional subfolder (inside JFrog Artifactory helm repository)" }, @@ -227,6 +206,28 @@ spec: } } } + # shellEvaluatedArguments lists the arguments that end up in a shell command, and where: "unquoted", or "double-quoted" + # when the command has the argument inside double quotes. The entrypoint script of the image builds the command + # from the environment variables that stepsTemplate creates from the arguments. + # In these arguments the engine passes the Git variables, such as CF_BRANCH, to the shell as environment variables, + # so that the shell doesn't run commands from a branch name or a commit message. + # Update the list when you change how an argument is used (see ShellArguments in the planner). + shellEvaluatedArguments: + kube_context: double-quoted + chart_name: unquoted + release_name: unquoted + namespace: unquoted + tiller_namespace: unquoted + chart_version: unquoted + app_version: unquoted + chart_repo_url: unquoted + custom_value_files: unquoted + custom_values: unquoted + cmd_ps: unquoted + set_file: unquoted + timeout: unquoted + commands: unquoted + chart_subdir: unquoted stepsTemplate: |- main: name: helm From 54d798c3242c637fd9a3ba72c77a5e1fe03df516 Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Tue, 10 Nov 2026 15:40:01 +0200 Subject: [PATCH 6/6] update branch, bump version --- graduated/codefresh-run/step.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graduated/codefresh-run/step.yaml b/graduated/codefresh-run/step.yaml index 75b1e406f..a0da36eeb 100644 --- a/graduated/codefresh-run/step.yaml +++ b/graduated/codefresh-run/step.yaml @@ -3,7 +3,7 @@ kind: step-type metadata: name: codefresh-run title: Run a Codefresh pipeline - version: 1.6.1 + version: 1.6.2 isPublic: true description: Run a Codefresh pipeline by ID or name and attach the created build logs. sources: