Improve GitHub Actions#20093
Conversation
This prevents storing a GITHUB_TOKEN locally such that scripts can perform authenticated git commands. Let's not do that.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (4)
WalkthroughThis PR pins third-party GitHub Actions to specific commit SHAs across CI, integration-tests, prepare-release, and release workflows, disables or removes pnpm package-manager caching in Node setup steps, adjusts/remove some Cargo/oxide cache steps, sets prepare job permissions and checkout options, and replaces the softprops release action with an explicit 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/release.yml (1)
297-311:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame
matrix.targetissue in oxide cache key.Proposed fix
- key: ${{ runner.os }}-${{ matrix.target }}-oxide-${{ hashFiles('./crates/**/*') }} + key: ${{ runner.os }}-oxide-${{ hashFiles('./crates/**/*') }}🤖 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/release.yml around lines 297 - 311, The "Cache oxide build" step's cache key uses ${{ matrix.target }} which doesn't exist in this job's matrix; update the step so the key references the actual matrix variable used for the oxide build (or move this step into the job that defines the matrix), e.g. replace ${{ matrix.target }} with the correct matrix identifier (such as the matrix entry that holds the build target name) or remove it and rely on runner.os+hashFiles to avoid a non-existent variable; target the key expression in the "Cache oxide build" step (the key: ${{ runner.os }}-${{ matrix.target }}-oxide-${{ hashFiles('./crates/**/*') }}) when making the change..github/workflows/prepare-release.yml (1)
277-291:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame
matrix.targetissue in oxide cache key.Apply the same fix as the cargo cache above.
Proposed fix
- key: ${{ runner.os }}-${{ matrix.target }}-oxide-${{ hashFiles('./crates/**/*') }} + key: ${{ runner.os }}-oxide-${{ hashFiles('./crates/**/*') }}🤖 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/prepare-release.yml around lines 277 - 291, The cache step named "Cache oxide build" currently uses a key containing "${{ matrix.target }}" which causes cache fragmentation; update the step so the "key" for the actions/cache entry removes the matrix.target segment and matches the cargo cache fix (e.g., use "${{ runner.os }}-oxide-${{ hashFiles('./crates/**/*') }}" or the same stable key pattern used in the cargo cache step), keeping the same "path" and "lookup-only" fields and leaving restore/restore-keys behavior consistent with the cargo cache change.
🤖 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/prepare-release.yml:
- Around line 265-274: The cache key in the "Cache cargo" step uses `${{
matrix.target }}` but the prepare job has no matrix, so `matrix.target` is
empty; update the `key` expression in the "Cache cargo" step (the step named
"Cache cargo" that contains the `key:` entry) to remove or replace
`matrix.target` (e.g., use `${{ runner.os }}` or a static token) so the cache
key is stable — e.g. change it to use just `${{ runner.os }}-cargo-${{
hashFiles('**/Cargo.lock') }}` or an equivalent single-job-safe value.
- Around line 350-364: The `Prepare GitHub Release` step runs `gh release create
${TAG_NAME}` unauthenticated; set the GitHub token for the step so `gh` can
authenticate. Update the step (named "Prepare GitHub Release") to provide
GITHUB_TOKEN (or GH_TOKEN) from the workflow secrets (e.g. secrets.GITHUB_TOKEN)
in the step's env, so the existing `gh release create ${TAG_NAME} --title
"${TAG_NAME}" --notes "${RELEASE_NOTES}" ...` invocation can succeed.
In @.github/workflows/release.yml:
- Around line 285-294: The cache key in the "Cache cargo" step uses ${{
matrix.target }} but the release job has no matrix, so replace the undefined
matrix reference with a stable identifier (e.g. remove matrix.target and use ${{
runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} or use an explicit job
env/var like ${{ env.TARGET }} if you introduce one); update the cache key in
the "Cache cargo" step and any other places referencing matrix.target in the
release job (and mirror the same change you applied in prepare-release.yml) so
the key no longer relies on a non-existent matrix.
---
Outside diff comments:
In @.github/workflows/prepare-release.yml:
- Around line 277-291: The cache step named "Cache oxide build" currently uses a
key containing "${{ matrix.target }}" which causes cache fragmentation; update
the step so the "key" for the actions/cache entry removes the matrix.target
segment and matches the cargo cache fix (e.g., use "${{ runner.os }}-oxide-${{
hashFiles('./crates/**/*') }}" or the same stable key pattern used in the cargo
cache step), keeping the same "path" and "lookup-only" fields and leaving
restore/restore-keys behavior consistent with the cargo cache change.
In @.github/workflows/release.yml:
- Around line 297-311: The "Cache oxide build" step's cache key uses ${{
matrix.target }} which doesn't exist in this job's matrix; update the step so
the key references the actual matrix variable used for the oxide build (or move
this step into the job that defines the matrix), e.g. replace ${{ matrix.target
}} with the correct matrix identifier (such as the matrix entry that holds the
build target name) or remove it and rely on runner.os+hashFiles to avoid a
non-existent variable; target the key expression in the "Cache oxide build" step
(the key: ${{ runner.os }}-${{ matrix.target }}-oxide-${{
hashFiles('./crates/**/*') }}) when making the change.
🪄 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
Run ID: f24870db-2fdc-4aee-a2b9-aa0fd6fe16bf
📒 Files selected for processing (4)
.github/workflows/ci.yml.github/workflows/integration-tests.yml.github/workflows/prepare-release.yml.github/workflows/release.yml
Confidence Score: 5/5All changes are defence-in-depth hardening of GitHub Actions; no functional behaviour is altered and the previous review concerns (GH_TOKEN, TAG_NAME quoting, registry-url) are all correctly addressed in the final diff. Every change is a security improvement — SHA pinning, credential isolation, cache removal from release jobs, and shell-variable substitution in run blocks. No logic is restructured, no auth path is removed, and registry-url is preserved for npm publishing. The diff introduces no new defects. No files require special attention. Reviews (4): Last reviewed commit: "rmeove caches from {prepare-}release.yml..." | Re-trigger Greptile |
702665b to
c2c22aa
Compare
To prevent cache poisoning attacks
The shell itself will perform variable expansion for us.
c2c22aa to
e5fc24a
Compare
This is auto-discovered baseed on the `packageManager` field. Let's use that as the source of truth
e5fc24a to
8fc6ebf
Compare
This PR improves GitHub workflows by making sure that:
env.…inrunblocksgh releaseinstead ofsoftprops/action-gh-release