fix(lcm): point stable latest at the promoted version digest - #2088
Conversation
📝 WalkthroughWalkthroughThe stable promotion workflow now copies only the requested image tag, assigns that promoted digest to ChangesStable image promotion
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
stable/<image>:latest was copied independently from staging/<image>:latest, This PR tags latest from the stable version tag that was just promoted instead, so the two are the same manifest by construction, and assert the digests match before the step succeeds. Also add set -euo pipefail so a crane failure mid-loop fails the step rather than being masked by the loop's last command.
ca0fffc to
fffd2fb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/promote-to-stable.yaml (1)
59-63: 🗄️ Data Integrity & Integration | 🔵 TrivialSerialize stable image promotions.
This workflow does not declare a GitHub Actions
concurrencygroup;group: infra1-runners-arcis only the runner selector. When two promotions overlap,crane tag stable/${image}:latestcan race with the digest checks, so an invocation can pass thepromoted/currentcomparison while another run retagslatest, leaving stable inconsistent. Add a stable-image promotion concurrency group, e.g. for each image/tag being promoted, or serialize these promotions.🤖 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 59 - 63, Add a GitHub Actions concurrency configuration to the stable image promotion workflow, using a group keyed by the image/tag being promoted or a shared group that serializes all stable promotions. Keep the existing runner selection unchanged, and ensure overlapping invocations cannot race between the crane tag operation and the promoted/current digest comparison.
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/workflows/promote-to-stable.yaml:
- Around line 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.
- Around line 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.
- Around line 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.
---
Nitpick comments:
In @.github/workflows/promote-to-stable.yaml:
- Around line 59-63: Add a GitHub Actions concurrency configuration to the
stable image promotion workflow, using a group keyed by the image/tag being
promoted or a shared group that serializes all stable promotions. Keep the
existing runner selection unchanged, and ensure overlapping invocations cannot
race between the crane tag operation and the promoted/current digest comparison.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 084c8c60-9076-4a32-b736-3408bea8693f
📒 Files selected for processing (1)
.github/workflows/promote-to-stable.yaml
| set -euo pipefail | ||
| built_images='${{ inputs.images }}' | ||
| tag=${{ inputs.image_tag }} | ||
| for image in $(echo "$built_images" | jq -r '.[]'); do |
There was a problem hiding this comment.
🎯 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" || trueRepository: 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.
| built_images='${{ inputs.images }}' | ||
| tag=${{ inputs.image_tag }} |
There was a problem hiding this comment.
🔒 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.yamlRepository: 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}")
PYRepository: 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:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections
- 2: https://github.blog/security/supply-chain-security/four-tips-to-keep-your-github-actions-workflows-secure/
- 3: https://docs.github.com/en/enterprise-cloud@latest/actions/concepts/security/script-injections
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
| 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 |
There was a problem hiding this comment.
🗄️ 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.
2 participants
stable/<image>:latestwas copied independently fromstaging/<image>:latest,This PR tags latest from the stable version tag that was just promoted instead, so the two are the same manifest by construction, and assert the digests match before the step succeeds. Also add
set -euo pipefailso a crane failure mid-loop fails the step rather than being masked by the loop's last command.Summary by CodeRabbit
latesttag from the same image digest.