ci: hash all depends recipe inputs - #7543
Conversation
WalkthroughThe depends build workflow now checks out the complete Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/build-depends.yml:
- Line 77: Update the CACHE_KEY_PREFIX assignment in the build-depends workflow
to use trusted internal output values for the base-image digest and runner
selection rather than interpolating caller-provided workflow inputs into the
Bash script; preserve the existing build-target component and cache-key format.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 82275f26-06c5-4e9d-a502-9b72e3744b45
📒 Files selected for processing (1)
.github/workflows/build-depends.yml
| DOCKERFILE_HASH="${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'contrib/containers/ci/ci-slim.Dockerfile') }}" | ||
| PACKAGES_HASH="${{ hashFiles('depends/packages/*', 'depends/Makefile') }}" | ||
| DEPENDS_INPUTS_HASH="${{ hashFiles('depends/Makefile', 'depends/funcs.mk', 'depends/gen_id', 'depends/config.guess', 'depends/config.sub', 'depends/config.site.in', 'depends/builders/**', 'depends/hosts/**', 'depends/packages/**', 'depends/patches/**') }}" | ||
| CACHE_KEY_PREFIX="depends-${DOCKERFILE_HASH}-${{ inputs.base-image-digest }}-${{ inputs.runs-on }}-${{ inputs.build-target }}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
workflow=".github/workflows/build-depends.yml"
echo "Workflow inputs and declarations:"
sed -n '1,45p' "$workflow"
echo
echo "Direct input interpolation:"
rg -n -C 3 '\$\{\{[[:space:]]*inputs\.(base-image-digest|runs-on|build-target)' "$workflow"
echo
echo "Callers and input sources:"
rg -n -C 6 'build-depends\.yml|base-image-digest|build-target|runs-on:' .github/workflows || trueRepository: dashpay/dash
Length of output: 50368
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "build.yml inputs and caller snippet:"
sed -n '1,120p' .github/workflows/build.yml
echo
echo "All build-depends usages:"
rg -n -C 4 'uses: ./.github/workflows/build-depands\.yml|uses: \./\.github/workflows/build-depends\.yml|build-depends\.yml|base-image-digest:|runs-on:|build-target:' .github/workflows/build.yml .github/workflows/cache-depends-sources.yml .github/workflows/build-container.yml || trueRepository: dashpay/dash
Length of output: 30887
🌐 Web query:
GitHub Actions template injection inputs context in shell environment variable source documentation
💡 Result:
Template injection in GitHub Actions occurs when untrusted data—such as issue titles, pull request bodies, or branch names—is directly interpolated into a workflow's run command using the ${{ }} expression syntax [1][2][3]. Because the GitHub Actions runner evaluates these expressions and substitutes them with raw text before executing the shell script, an attacker can supply malicious input containing shell control characters (e.g., quotes, semicolons, or backticks) to break out of the intended command and execute arbitrary code on the runner [1][4][3]. The authoritative mitigation for this vulnerability is to never place untrusted input directly within a run script [5][6]. Instead, you should pass the input into the step's environment variables and then reference that variable within your script [5][2][7]. Recommended pattern: - name: Securely handle untrusted input env: # Set the untrusted input to an environment variable UNTRUSTED_INPUT: ${{ github.event.issue.title }} run: | # Reference the variable using standard shell syntax echo "The input is: $UNTRUSTED_INPUT" By storing the input in an environment variable, the value is treated as data rather than as part of the script's template, which prevents the shell from interpreting the content as executable commands [2][4][7]. Additionally, always use double quotes when referencing environment variables in shell scripts to prevent word splitting and other unexpected shell behaviors [2][6][7]. Other security best practices include: - Applying the principle of least privilege by restricting GITHUB_TOKEN permissions [6][8][3]. - Using static analysis tools like CodeQL to detect potential injection sinks in your workflow files [3]. - Treating all contexts that accept external input (e.g., github.event, inputs) as potentially malicious [1][5].
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/script-injections
- 2: https://securitylab.github.com/resources/github-actions-untrusted-input/
- 3: https://github.blog/security/vulnerability-research/how-to-catch-github-actions-workflow-injections-before-attackers-do/
- 4: feat: auto-extract shell injection expressions from run: steps into env vars github/gh-aw#28998
- 5: https://github.blog/security/supply-chain-security/four-tips-to-keep-your-github-actions-workflows-secure/
- 6: https://docs.github.com/en/actions/reference/security/secure-use
- 7: https://docs.github.com/en/enterprise-server@2.22/actions/security-guides/security-hardening-for-github-actions
- 8: https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/GitHub_Actions_Security_Cheat_Sheet.md
Use output values instead of caller inputs in the shell script.
This reusable workflow receives inputs.* from the caller via workflow_call; passing them directly lets the caller supply shell metacharacters such as quotes, command substitution, or newlines in the generated Bash script. .github/workflows/build.yml already passes internal needs.check-skip.outputs.base-image-digest and needs.check-skip.outputs['runner-amd64'] values, so use those instead of .github/workflows/build.yml inputs in the shell source.
🧰 Tools
🪛 zizmor (1.28.0)
[error] 77-77: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 77-77: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 77-77: 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/build-depends.yml at line 77, Update the CACHE_KEY_PREFIX
assignment in the build-depends workflow to use trusted internal output values
for the base-image digest and runner selection rather than interpolating
caller-provided workflow inputs into the Bash script; preserve the existing
build-target component and cache-key format.
Source: Linters/SAST tools
|
🕓 Ready for review — 6 ahead in queue (commit 5950828) |
PastaPastaPasta
changed the title
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex + Sonnet
This is a small, well-verified CI-only change that expands the depends cache key to hash all recipe inputs (Makefile, funcs.mk, gen_id, config scripts, builders, hosts, packages, patches) instead of a narrow subset, and widens the sparse-checkout to fetch the full depends/ tree so hashFiles can see those inputs. I independently confirmed every referenced path exists in the repo and that the change achieves its stated goal of fixing stale-cache hits without altering build behavior. Both Codex and the retried Sonnet reviewer agree the PR is clean with no findings.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed) - Verifier:
claude-sonnet-5— final-verifier - Sonnet reviewers:
claude-sonnet-5— general (failed),claude-sonnet-5— dash-core-commit-history (failed),claude-sonnet-5— dash-core-commit-history (completed),claude-sonnet-5— general (completed)
df51875 to
5950828
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/build-depends.yml (1)
77-78: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winInjection (CWE-78): Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
Do not interpolate workflow inputs into the Bash source.
Line 77 inserts
inputs.base-image-digest,inputs.runs-on, andinputs.build-targetbefore Bash executes the script. A caller-supplied quote or command substitution can terminateCACHE_KEY_PREFIXand execute commands on the runner.Pass these values through
env:and reference quoted shell variables. Validate expected formats before using them. Apply the same fix to the directinputs.build-targetinterpolation on Line 68.Proposed fix
+ env: + INPUT_BASE_IMAGE_DIGEST: ${{ inputs.base-image-digest }} + INPUT_RUNS_ON: ${{ inputs.runs-on }} + INPUT_BUILD_TARGET: ${{ inputs.build-target }} run: | - BUILD_TARGET="${{ inputs.build-target }}" + BUILD_TARGET="${INPUT_BUILD_TARGET}" ... - CACHE_KEY_PREFIX="depends-${DOCKERFILE_HASH}-${{ inputs.base-image-digest }}-${{ inputs.runs-on }}-${{ inputs.build-target }}" + CACHE_KEY_PREFIX="depends-${DOCKERFILE_HASH}-${INPUT_BASE_IMAGE_DIGEST}-${INPUT_RUNS_ON}-${BUILD_TARGET}"#!/usr/bin/env bash set -euo pipefail echo "Workflow inputs:" sed -n '1,45p' .github/workflows/build-depends.yml echo echo "All callers and input sources:" rg -n -C 6 \ 'build-depends\.yml|base-image-digest:|runs-on:|build-target:' \ .github/workflows🤖 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/build-depends.yml around lines 77 - 78, Remove direct GitHub Actions input interpolation from the Bash source, including the build-target use near line 68 and CACHE_KEY_PREFIX construction. Pass base-image-digest, runs-on, and build-target through the step’s env block, reference them as quoted shell variables, and validate each value against its expected format before constructing CACHE_KEY_PREFIX and CACHE_KEY.Source: Linters/SAST tools
🤖 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.
Duplicate comments:
In @.github/workflows/build-depends.yml:
- Around line 77-78: Remove direct GitHub Actions input interpolation from the
Bash source, including the build-target use near line 68 and CACHE_KEY_PREFIX
construction. Pass base-image-digest, runs-on, and build-target through the
step’s env block, reference them as quoted shell variables, and validate each
value against its expected format before constructing CACHE_KEY_PREFIX and
CACHE_KEY.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c156111e-a36d-489e-a0f8-1198ee10307d
📒 Files selected for processing (1)
.github/workflows/build-depends.yml
3 participants
Issue being fixed or feature implemented
The depends cache key did not cover every file used to derive depends package build IDs. In run 30864594362, PR #7540 changed
depends/hosts/darwin.mk, but cache lookup still reported a hit and skipped the dedicated depends build. The macOS source job then rebuilt packages for about 17 minutes.What was done?
depends/directory in the cache-check job so those inputs are available tohashFiles.This causes a true cache miss when a relevant depends input changes, allowing the existing depends job/artifact handoff to provide matching packages to source jobs.
How Has This Been Tested?
git diff --check.actionlint .github/workflows/build-depends.yml. It reports existing diagnostics for the repository's custom checkout input and pre-existing shell-style issues; no diagnostics are caused by this change.Breaking Changes
None.
Checklist:
This pull request was created by Codex.