ci: fix attestation and security-deep workflow failures#101
Conversation
- Replace unpinned dtolnay/rust-toolchain@stable with bare rustup (fixes attestation SHA-pinning requirement) - Make Kani rustup conditional to avoid EXDEV in containers
📝 WalkthroughWalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (workflow_dispatch)"
participant GH as "GitHub Actions"
participant Validate as "validate-inputs job"
participant Jobs as "Dependent jobs (build/attest/publish)"
participant Git as "Git Remote (tags)"
rect rgba(200,220,255,0.5)
User->>GH: Trigger workflow with force_release + release_tag
end
rect rgba(220,255,200,0.5)
GH->>Validate: Start `validate-inputs`
Validate->>Git: Verify `release_tag` matches vX.Y.Z and exists remotely
Git-->>Validate: Tag exists / not found
Validate-->>GH: Export validated `release_tag` or fail workflow
end
rect rgba(255,230,200,0.5)
GH->>Jobs: Run dependent jobs only if previous jobs not failed/cancelled
Jobs->>GH: Use validated `release_tag` as checkout `ref` when provided
Jobs-->>GH: Proceed to `publish` if release created or force_release
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
The attest job only checked release_created, so manual workflow_dispatch with force_release=true would build wheels then skip attestation and publishing. Now both attest gate and checkout ref respect the force_release/release_tag inputs.
- Add validate-inputs job that fails fast when force_release=true but release_tag is empty (prevents publishing from wrong commit) - Add explicit if-guard to publish job (defense in depth) - Wire validate-inputs into build-wheels and build-sdist needs
![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 the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release-please.yml:
- Around line 58-63: Validate that the input release_tag matches the exact
semver tag pattern (e.g., regex ^cachekit-v[0-9]+\.[0-9]+\.[0-9]+$) and reject
others in the force_release check; then verify the tag actually exists on the
remote (use git ls-remote --tags origin refs/tags/<release_tag> and fail if no
match) before proceeding to any checkout. Also export the value as an
environment variable (e.g., RELEASE_TAG) and pass that env into subsequent steps
instead of embedding ${{ github.event.inputs.release_tag }} directly into
actions/checkout ref so actions/checkout uses ref: ${{ env.RELEASE_TAG }}
(update all checkout usages) to make the workflow safer.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 877d8d59-6387-4d99-8435-210cb142bd5c
📒 Files selected for processing (1)
.github/workflows/release-please.yml
| - name: Require release_tag for force_release | ||
| run: | | ||
| if [ -z "${{ github.event.inputs.release_tag }}" ]; then | ||
| echo "::error::force_release requires release_tag to be set (e.g., cachekit-v0.6.0)" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Confirm direct interpolation of workflow input inside shell run block:"
rg -n -C2 '\${{\s*github\.event\.inputs\.release_tag\s*}}' .github/workflows/release-please.yml
echo
echo "2) Confirm whether tag existence is validated (expected: no git tag existence check today):"
rg -n -C3 'Require release_tag|ls-remote|refs/tags|release_tag must' .github/workflows/release-please.ymlRepository: cachekit-io/cachekit-py
Length of output: 1789
🏁 Script executed:
#!/bin/bash
# Attempt to read the release-please.yml file
if [ -f ".github/workflows/release-please.yml" ]; then
echo "=== File exists, reading lines 50-70 ==="
sed -n '50,70p' .github/workflows/release-please.yml
echo
echo "=== Full validation section ==="
sed -n '/validate-inputs/,/^[^ ]/p' .github/workflows/release-please.yml | head -30
else
echo "File not found at .github/workflows/release-please.yml"
find . -name "release-please.yml" 2>/dev/null | head -5
fiRepository: cachekit-io/cachekit-py
Length of output: 2476
🏁 Script executed:
#!/bin/bash
# Search for where release_tag is used in the workflow
echo "=== Searching for all uses of 'release_tag' in the workflow ==="
grep -n "release_tag" .github/workflows/release-please.yml
echo
echo "=== Check if tag is used for git checkout or similar operations ==="
grep -n -A5 -B5 "checkout\|git\|ref:" .github/workflows/release-please.yml | head -50Repository: cachekit-io/cachekit-py
Length of output: 3203
Harden release_tag validation before using it in git checkout.
Line 60 only validates that release_tag is non-empty, but the input is passed directly to actions/checkout with ref: ${{ ... }} at lines 99, 134, and 176. This allows arbitrary branches, SHAs, or nonexistent refs to be checked out, causing the build to use wrong code. Add format validation (must match cachekit-vX.Y.Z) and verify the tag exists on the remote before allowing the workflow to proceed.
Additionally, pass release_tag via env: instead of inline ${{ ... }} interpolation in the run block as a defensive best practice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release-please.yml around lines 58 - 63, Validate that the
input release_tag matches the exact semver tag pattern (e.g., regex
^cachekit-v[0-9]+\.[0-9]+\.[0-9]+$) and reject others in the force_release
check; then verify the tag actually exists on the remote (use git ls-remote
--tags origin refs/tags/<release_tag> and fail if no match) before proceeding to
any checkout. Also export the value as an environment variable (e.g.,
RELEASE_TAG) and pass that env into subsequent steps instead of embedding ${{
github.event.inputs.release_tag }} directly into actions/checkout ref so
actions/checkout uses ref: ${{ env.RELEASE_TAG }} (update all checkout usages)
to make the workflow safer.
- Validate semver pattern (vX.Y.Z) and verify tag exists on remote - Route validated tag through job output instead of raw input - Fix example in input description (v0.6.0, not cachekit-v0.1.0)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release-please.yml (1)
162-197:⚠️ Potential issue | 🟠 MajorAdd the same status guard to attestation.
This job still depends on
validate-inputs, which is intentionally skipped on the normal release path. Without!failure()/!cancelled()here, attestation may never start for standard releases.🔒 Proposed fix
attest: name: Attest Build Provenance and SBOM needs: [release-please, validate-inputs, build-wheels, build-sdist] - if: needs.release-please.outputs.release_created == 'true' || github.event.inputs.force_release == 'true' + if: ${{ !failure() && !cancelled() && (needs.release-please.outputs.release_created == 'true' || github.event.inputs.force_release == 'true') }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-please.yml around lines 162 - 197, The "Attest Build Provenance" step lacks the job status guard and can be skipped when validate-inputs is intentionally omitted; update the step identified by its name "Attest Build Provenance" (uses: actions/attest-build-provenance@...) to include an if condition: (needs.release-please.outputs.release_created == 'true' || github.event.inputs.force_release == 'true') && !failure() && !cancelled() so the attestation only runs for the intended release path and only when the job hasn't failed or been cancelled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release-please.yml:
- Around line 52-76: The validate-inputs job's validate step uses git ls-remote
--tags origin "$TAG" but no git remote is configured because there's no checkout
in this job; update the validate step (id: validate) to either configure the
remote before checking (e.g., run git init && git remote add origin
"https://github.com/${{ github.repository }}.git") or replace the ls-remote
check with a GitHub API call using GITHUB_REPOSITORY and GITHUB_TOKEN to verify
the tag exists; ensure the validation still writes release_tag to GITHUB_OUTPUT
when the tag is found.
---
Outside diff comments:
In @.github/workflows/release-please.yml:
- Around line 162-197: The "Attest Build Provenance" step lacks the job status
guard and can be skipped when validate-inputs is intentionally omitted; update
the step identified by its name "Attest Build Provenance" (uses:
actions/attest-build-provenance@...) to include an if condition:
(needs.release-please.outputs.release_created == 'true' ||
github.event.inputs.force_release == 'true') && !failure() && !cancelled() so
the attestation only runs for the intended release path and only when the job
hasn't failed or been cancelled.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e43d5ac-26d5-4b16-a806-c04dbfef2fef
📒 Files selected for processing (1)
.github/workflows/release-please.yml
Use full repository URL instead of 'origin' alias since the validate-inputs job has no actions/checkout step.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/release-please.yml (1)
63-63:⚠️ Potential issue | 🟠 MajorAvoid direct expression interpolation of
release_taginsiderun.Line 63 injects
${{ github.event.inputs.release_tag }}directly into shell source. Pass it throughenv:and read it as a normal shell variable to avoid injection/parsing edge cases.🔧 Proposed fix
steps: - name: Validate release_tag id: validate + env: + RELEASE_TAG: ${{ github.event.inputs.release_tag }} run: | - TAG="${{ github.event.inputs.release_tag }}" + TAG="$RELEASE_TAG" if [ -z "$TAG" ]; then echo "::error::force_release requires release_tag (e.g., v0.6.0)" exit 1#!/bin/bash # Verify direct expression interpolation inside shell run blocks. rg -n -C2 'TAG="\$\{\{\s*github\.event\.inputs\.release_tag\s*\}\}"|RELEASE_TAG' .github/workflows/release-please.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-please.yml at line 63, The run step currently injects the GitHub input directly into the shell via TAG="${{ github.event.inputs.release_tag }}"; change this to set the release_tag in the job/step env (e.g., env: RELEASE_TAG: ${{ github.event.inputs.release_tag }}) and then reference the shell variable inside the run block (use $RELEASE_TAG or $TAG) so the expression is not interpolated directly in the shell; update any references to TAG in the run script to use the chosen shell variable name (RELEASE_TAG/TAG) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release-please.yml:
- Around line 55-56: The validate-inputs job is currently conditional (if:
github.event.inputs.force_release == 'true') but is declared as a dependency for
build jobs (needs: validate-inputs) and its output release_tag is later accessed
(needs.validate-inputs.outputs.release_tag), which causes dependent jobs to be
skipped or have undefined outputs when validate-inputs is skipped; change the
validate-inputs job to always run (e.g., if: ${{ always() }}) so it always
supplies outputs, and update downstream jobs that list needs: validate-inputs
(the build jobs and any job reading needs.validate-inputs.outputs.release_tag)
to explicitly check validate-inputs result (condition e.g., if:
needs.validate-inputs.result == 'success' or guard usage of
needs.validate-inputs.outputs.release_tag) before proceeding.
---
Duplicate comments:
In @.github/workflows/release-please.yml:
- Line 63: The run step currently injects the GitHub input directly into the
shell via TAG="${{ github.event.inputs.release_tag }}"; change this to set the
release_tag in the job/step env (e.g., env: RELEASE_TAG: ${{
github.event.inputs.release_tag }}) and then reference the shell variable inside
the run block (use $RELEASE_TAG or $TAG) so the expression is not interpolated
directly in the shell; update any references to TAG in the run script to use the
chosen shell variable name (RELEASE_TAG/TAG) accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7172d7ff-ad9d-4e24-a893-95eb9cda5111
📒 Files selected for processing (1)
.github/workflows/release-please.yml
| if: github.event.inputs.force_release == 'true' | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In GitHub Actions, if job B has needs: [A]and job A is skipped by anifcondition, does job B run by default without usingalways()?
💡 Result:
No, job B does not run by default if job A is skipped due to an if condition. Jobs with needs require their dependencies to complete successfully; a skipped prerequisite causes the dependent job to be skipped unless an explicit conditional like always or !failure && !cancelled is used in the dependent job's if condition.
Citations:
- 1: https://docs.github.com/actions/using-jobs/using-jobs-in-a-workflow
- 2: https://docs.github.com/en/actions/using-jobs/using-jobs-in-a-workflow
- 3: https://docs.github.com/actions/reference/workflows-and-actions/workflow-syntax
🏁 Script executed:
cd .github/workflows && ls -la release-please.ymlRepository: cachekit-io/cachekit-py
Length of output: 135
🏁 Script executed:
cat -n .github/workflows/release-please.yml | sed -n '50,90p'Repository: cachekit-io/cachekit-py
Length of output: 2187
🏁 Script executed:
cat -n .github/workflows/release-please.yml | sed -n '135,150p'Repository: cachekit-io/cachekit-py
Length of output: 833
🏁 Script executed:
cat -n .github/workflows/release-please.yml | head -20Repository: cachekit-io/cachekit-py
Length of output: 615
validate-inputs as a hard dependency will block normal release-please runs.
validate-inputs only runs when force_release == 'true' (line 55), but both build jobs always need it (lines 80, 141). In GitHub Actions, skipped dependencies automatically skip their dependent jobs, preventing regular releases from building even when release_created == 'true'. Additionally, line 147 attempts to access needs.validate-inputs.outputs.release_tag which will be undefined when the job is skipped.
The proposed fix pattern is correct: make validate-inputs always run (e.g., if: ${{ always() }}) and update dependent jobs to explicitly check that validate-inputs completed successfully before proceeding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release-please.yml around lines 55 - 56, The
validate-inputs job is currently conditional (if:
github.event.inputs.force_release == 'true') but is declared as a dependency for
build jobs (needs: validate-inputs) and its output release_tag is later accessed
(needs.validate-inputs.outputs.release_tag), which causes dependent jobs to be
skipped or have undefined outputs when validate-inputs is skipped; change the
validate-inputs job to always run (e.g., if: ${{ always() }}) so it always
supplies outputs, and update downstream jobs that list needs: validate-inputs
(the build jobs and any job reading needs.validate-inputs.outputs.release_tag)
to explicitly check validate-inputs result (condition e.g., if:
needs.validate-inputs.result == 'success' or guard usage of
needs.validate-inputs.outputs.release_tag) before proceeding.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Move github.event.inputs.release_tag from direct ${{ }} shell
interpolation to step env, avoiding potential script injection
from crafted workflow_dispatch inputs.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/release-please.yml (1)
163-165:⚠️ Potential issue | 🟠 Major
attestcan be skipped on normal releases due to skipped dependency.Line 163 includes
validate-inputsinneeds, but Line 164 does not use the same status-function guard as build/publish. Whenforce_releaseis false,validate-inputsis skipped, andattestmay be skipped unexpectedly.🔧 Suggested fix
- if: needs.release-please.outputs.release_created == 'true' || github.event.inputs.force_release == 'true' + if: ${{ !failure() && !cancelled() && (needs.release-please.outputs.release_created == 'true' || github.event.inputs.force_release == 'true') }}In GitHub Actions, if a job has `needs` on a job that was skipped, does adding `if: ${{ !failure() && !cancelled() }}` allow it to run, compared with an `if` that has no status-check function?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-please.yml around lines 163 - 165, The attest job currently lists needs: [validate-inputs, release-please, build-wheels, build-sdist] but lacks the same status-check guard used by the release/publish path, so attest can be skipped when validate-inputs is skipped; update the attest job's if expression (the one referencing needs.release-please.outputs.release_created and github.event.inputs.force_release) to include the status-function guard (e.g. append && !failure() && !cancelled()) so the job will run when appropriate even if a prior needed job was skipped; locate the attest job and modify its if condition accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/release-please.yml:
- Around line 163-165: The attest job currently lists needs: [validate-inputs,
release-please, build-wheels, build-sdist] but lacks the same status-check guard
used by the release/publish path, so attest can be skipped when validate-inputs
is skipped; update the attest job's if expression (the one referencing
needs.release-please.outputs.release_created and
github.event.inputs.force_release) to include the status-function guard (e.g.
append && !failure() && !cancelled()) so the job will run when appropriate even
if a prior needed job was skipped; locate the attest job and modify its if
condition accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 55a37a90-ccc4-4830-bbb2-fac03b7f53c4
📒 Files selected for processing (1)
.github/workflows/release-please.yml
1 participant
Summary
dtolnay/rust-toolchain@stablewith barerustupin attestation job — fixes SHA-pinning enforcement that blocked the 0.6.0 releaserustup toolchain installconditional (skip if stable already present) — avoidsEXDEVcross-device link error in containers (rust-lang/rustup#1239)Test plan
Summary by CodeRabbit
Chores
Rust setup