Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions .github/workflows/promote-to-stable.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,24 @@ jobs:

- name: Promote images
run: |
set -euo pipefail
built_images='${{ inputs.images }}'
tag=${{ inputs.image_tag }}
Comment on lines 52 to 53

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

rg -n -C 6 'workflow_dispatch|workflow_call|pull_request|inputs\.(images|image_tag)' .github/workflows/promote-to-stable.yaml

Repository: gooddata/gooddata-ruby

Length of output: 989


🏁 Script executed:

python3 - <<'PY'
from pathlib import Path
path = Path(".github/workflows/promote-to-stable.yaml")
text = path.read_text()
print("file exists:", path.exists())
for i, line in enumerate(text.splitlines(), 1):
    print(f"{i:03d}: {line}")
PY

Repository: gooddata/gooddata-ruby

Length of output: 3465


🌐 Web query:

GitHub Actions workflow_call inputs expression evaluation substituted into run string before execution

💡 Result:

Yes, GitHub Actions expressions wrapped in ${{ }} syntax are evaluated and substituted into the run command string before the shell script is executed on the runner [1][2][3]. When you use the ${{ }} syntax directly within a run step, the GitHub Actions runner processes the expression, converts it into its string representation, and performs a literal text substitution into the command block [1][3]. This occurs before the runner generates and executes the temporary shell script [1][3]. Security Implications Because this substitution happens before the shell interprets the command, using ${{ }} for inputs directly inside a run string is a primary vector for shell command injection [1][2]. An attacker who controls the input value can inject characters (such as ", ;, or &) to break out of intended quotes or command structures, allowing them to execute arbitrary commands on the runner [1][3]. Recommended Best Practice To safely handle inputs in run steps, you should use environment variables instead of direct ${{ }} substitution [2]. By mapping the input to an environment variable in the env block, the value is passed to the shell process safely as data rather than being interpreted as part of the command's raw text [2]. Example of the vulnerable approach: - name: Vulnerable step run: echo "${{ inputs.my-input }}" # Unsafe if input contains shell metacharacters Example of the recommended approach: - name: Secure step env: MY_INPUT: ${{ inputs.my-input }} run: echo "$MY_INPUT" # Safe, as the value is treated as a variable string

Citations:


Do not interpolate workflow inputs directly into shell source.

${{ inputs.image_tag }} is substituted into the runner script before Bash executes it, so a tag containing shell metacharacters can inject commands during image promotion. Map both inputs.images and inputs.image_tag via step-level env, quote the shell variables, and validate IMAGE_TAG before using it as a registry tag.

🧰 Tools
🪛 zizmor (1.28.0)

[error] 52-52: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)


[error] 53-53: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/promote-to-stable.yaml around lines 52 - 53, Update the
promotion step around built_images and tag to pass inputs.images and
inputs.image_tag through step-level environment variables, then reference those
quoted shell variables instead of interpolating workflow expressions into Bash.
Validate IMAGE_TAG against the allowed registry-tag format before using it, and
preserve the existing image promotion behavior for valid inputs.

Source: Linters/SAST tools

for image in $(echo "$built_images" | jq -r '.[]'); do
Comment on lines +51 to 54

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

bash -c 'set -euo pipefail; for image in $(printf "%s" "{bad}" | jq -r ".[]"); do :; done; echo survived'

Repository: gooddata/gooddata-ruby

Length of output: 230


🏁 Script executed:

#!/bin/bash
set -u

# Inspect the workflow around the reported lines and any relevant context.
if [ -f ".github/workflows/promote-to-stable.yaml" ]; then
  echo "== file exists =="
  wc -l ".github/workflows/promote-to-stable.yaml"
  echo "== lines 1-90 =="
  cat -n ".github/workflows/promote-to-stable.yaml" | sed -n '1,90p'
else
  echo "file not found"
fi

echo "== bash pipe/subshell behavior probe =="
bash -c 'set -euo pipefail; for image in $(printf "%s" "{bad}" | jq -r ".[]"); do :; done; echo survived' || true
echo "== bash function-local set vs pipeline probe =="
bash -c 'set -euo pipefail
run() { for image in $(printf "%s" "{bad}" | jq -r ".[]"); do :; done; echo survived_after_function; }
run
echo survived_after_call' || true
echo "== bash -c exit probe =="
bash -c "set -euo pipefail; for image in \$(printf '%s' '{bad}' | jq -r '.[]'); do :; done; echo survived" || true

Repository: gooddata/gooddata-ruby

Length of output: 4050


Capture the jq status before iterating.

jq parses {bad} and exits non-zero, but the $(...) subshell status is not observed by the for list, so set -euo pipefail does not fail the promotion step. Save jq output/status before for, then loop over captured results.

🧰 Tools
🪛 zizmor (1.28.0)

[error] 52-52: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)


[error] 53-53: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/promote-to-stable.yaml around lines 51 - 54, Update the
image-processing block around the built_images and for loop to execute jq
separately, capture its output and exit status, and fail the step when parsing
fails. Iterate over the captured jq results only after validating the status,
preserving the existing image handling behavior.

crane cp \
"${INFRA_REPO_URL}/staging/${image}:${tag}" \
"${INFRA_REPO_URL}/stable/${image}:${tag}"
crane cp \
"${INFRA_REPO_URL}/staging/${image}:latest" \
"${INFRA_REPO_URL}/stable/${image}:latest"
# latest must be the digest just promoted, not whatever staging:latest is now
crane tag "${INFRA_REPO_URL}/stable/${image}:${tag}" latest

promoted=$(crane digest "${INFRA_REPO_URL}/stable/${image}:${tag}")
current=$(crane digest "${INFRA_REPO_URL}/stable/${image}:latest")
if [[ "${promoted}" != "${current}" ]]; then
echo "::error::stable/${image}:latest (${current}) != :${tag} (${promoted})"
exit 1
fi
Comment on lines +61 to +66

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Verify the staging digest as well.

The current check only compares stable/${image}:${tag} with stable/${image}:latest; it never verifies the stated staging-to-stable digest contract. Capture the staging digest before copying and include it in the comparison.

Proposed fix
+            staging=$(crane digest "${INFRA_REPO_URL}/staging/${image}:${tag}")
             crane cp \
               "${INFRA_REPO_URL}/staging/${image}:${tag}" \
               "${INFRA_REPO_URL}/stable/${image}:${tag}"
...
-            if [[ "${promoted}" != "${current}" ]]; then
+            if [[ "${staging}" != "${promoted}" || "${promoted}" != "${current}" ]]; then
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/promote-to-stable.yaml around lines 61 - 66, Update the
promotion verification around the promoted and current digest checks to capture
the corresponding staging image digest before copying, then compare that staging
digest with the stable tagged digest and retain the latest-tag comparison.
Include the relevant staging and stable references in any mismatch error while
preserving the existing failure behavior.


echo "Promoted image ${image} to stable"
echo "Promoted image ${image} to stable as ${tag} + latest"
done

- name: Promote charts
Expand Down
Loading