Skip to content

implement hip-15 for helm chart [WPB-27901] - #22167

Merged
e-maad merged 2 commits into
release/q2-2025-govfrom
q2-2025-gov-hip-15
Aug 11, 2026
Merged

e-maad merged 2 commits into
release/q2-2025-govfrom
q2-2025-gov-hip-15

Conversation

@smatting

Copy link
Copy Markdown
Contributor

@semgrep-code-wireapp

Copy link
Copy Markdown

Semgrep found 1 run-shell-injection finding:

Using variable interpolation ${{...}} with github context data in a run: step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code. github context data can have arbitrary user input and should be treated as untrusted. Instead, use an intermediate environment variable with env: to store the data and use the environment variable in the run: script. Be sure to use double-quotes the environment variable, like this: "$ENVVAR".

@smatting
smatting enabled auto-merge (squash) August 11, 2026 12:43
@e-maad e-maad changed the title implement hip-15 for helm chart implement hip-15 for helm chart [WPB-27901] Aug 11, 2026
Comment on lines 156 to 175
@@ -168,8 +167,9 @@ jobs:
fi
echo "chart_version=$chart_version" >> $GITHUB_OUTPUT

chart_patched="$(yq -Mr ".version = \"$chart_version\" | .appVersion = \"$image_tag\"" ./charts/webapp/Chart.yaml)"
chart_patched="$(yq -Mr ".version = \"$chart_version\" | .appVersion = \"$IMAGE_TAG\"" ./charts/webapp/Chart.yaml)"
echo "$chart_patched" > ./charts/webapp/Chart.yaml
sed -i "s/:do-not-use/:$IMAGE_TAG/g" ./charts/webapp/Chart.yaml

helm package ./charts/webapp

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semgrep identified a blocking 🔴 issue in your code:

Direct interpolation of github.ref in the shell script allows code injection if an attacker controls the branch or tag name, potentially exposing secrets and compromising the build pipeline.

More details about this

The run: step uses ${{ github.ref }} directly in a bash conditional without proper escaping. An attacker who controls a branch name or tag could inject arbitrary commands that execute in the runner.

Exploit scenario:

  1. An attacker creates a branch named "; rm -rf / # (or similar malicious payload).
  2. When this workflow runs on that branch, github.ref contains the attacker's branch name.
  3. The line if [[ "${{github.ref}}" =~ "/refs/tags" ]]; then expands to something like if [[ "; rm -rf / #" =~ "/refs/tags" ]]; then, which breaks out of the intended conditional and executes the injected command rm -rf /.
  4. The attacker's code now runs with the permissions of the GitHub Actions runner, giving them access to all secrets (AWS credentials, Quay passwords) in the env: section and the ability to steal code or modify artifacts.

While this specific case involves a regex test (which is safer than direct command substitution), the pattern is flagged because untrusted github context data passed directly to shell scripts can be exploited in other contexts within the same run: block or in future modifications.

To resolve this comment:

✨ Commit fix suggestion

Suggested change
env:
AWS_ACCESS_KEY_ID: ${{ secrets.CHARTS_WEBAPP_AUTOMATION_AWS_ACCESS_KEY_ID }}
AWS_SECRET_ACCESS_KEY: ${{ secrets.CHARTS_WEBAPP_AUTOMATION_AWS_SECRET_ACCESS_KEY }}
IMAGE_TAG: ${{ steps.push_docker_image.outputs.image_tag }}
GITHUB_REF_INPUT: ${{ github.ref }}
run: |
set -eo pipefail
helm plugin install https://github.com/hypnoglow/helm-s3.git --version 0.15.1
helm repo add charts-webapp s3://public.wire.com/charts-webapp
if [[ "${GITHUB_REF_INPUT}" =~ /refs/tags ]]; then
chart_version="$(./bin/chart-next-version.sh release)"
else
chart_version="$(./bin/chart-next-version.sh prerelease)"
fi
echo "chart_version=$chart_version" >> $GITHUB_OUTPUT
chart_patched="$(yq -Mr ".version = \"$chart_version\" | .appVersion = \"$IMAGE_TAG\"" ./charts/webapp/Chart.yaml)"
echo "$chart_patched" > ./charts/webapp/Chart.yaml
sed -i "s/:do-not-use/:$IMAGE_TAG/g" ./charts/webapp/Chart.yaml
helm package ./charts/webapp
helm s3 push --relative webapp-*.tgz charts-webapp
# Push to Quay OCI registry
echo "Logging in to Quay with Helm..."
echo "${{ secrets.WEBTEAM_QUAY_PASSWORD }}" | helm registry login -u "${{ secrets.WEBTEAM_QUAY_USERNAME }}" --password-stdin quay.io
echo "Pushing chart to OCI Quay..."
helm_output=$(helm push ./webapp-*.tgz oci://quay.io/wire/charts/dev 2>&1) || {
echo "Helm push to Quay failed with exit code $?"
echo "Output: $helm_output"
exit 1
}
echo "$helm_output"
digest=$(echo "$helm_output" | grep -oP 'sha256:\K[a-f0-9]{64}' | head -1)
echo "chart_digest=sha256:$digest" >> $GITHUB_OUTPUT
View step-by-step instructions
  1. Add an environment variable for the untrusted GitHub context value on this step, for example GITHUB_REF_INPUT: ${{ github.ref }} under the existing env: block.
  2. Replace the direct expression inside the shell script with the environment variable, changing if [[ "${{github.ref}}" =~ "/refs/tags" ]]; then to if [[ "${GITHUB_REF_INPUT}" =~ /refs/tags ]]; then.
  3. Keep the environment variable wrapped in double quotes everywhere you read it in run: so shell metacharacters are treated as data instead of code.
💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by run-shell-injection.

You can view more details about this finding in the Semgrep AppSec Platform.

@e-maad
e-maad disabled auto-merge August 11, 2026 13:48
@e-maad
e-maad merged commit b9083dd into release/q2-2025-gov Aug 11, 2026
6 of 8 checks passed
@e-maad
e-maad deleted the q2-2025-gov-hip-15 branch August 11, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants