Skip to content

Improve GitHub Actions#20093

Merged
RobinMalfait merged 8 commits into
mainfrom
chore/improve-actions
May 21, 2026
Merged

Improve GitHub Actions#20093
RobinMalfait merged 8 commits into
mainfrom
chore/improve-actions

Conversation

@RobinMalfait

@RobinMalfait RobinMalfait commented May 21, 2026

Copy link
Copy Markdown
Member

This PR improves GitHub workflows by making sure that:

  • Uesed actions are pinned
  • We don't persist credentials
  • Remove caches from release workflows
  • Prevent template expansion when using env.… in run blocks
  • Use the gh release instead of softprops/action-gh-release

This prevents storing a GITHUB_TOKEN locally such that scripts can
perform authenticated git commands. Let's not do that.
@RobinMalfait RobinMalfait requested a review from a team as a code owner May 21, 2026 09:18
@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: bd844a26-83b0-4a70-a12a-45aa3f7e9ad0

📥 Commits

Reviewing files that changed from the base of the PR and between e5fc24a and 8fc6ebf.

📒 Files selected for processing (4)
  • .github/workflows/ci.yml
  • .github/workflows/integration-tests.yml
  • .github/workflows/prepare-release.yml
  • .github/workflows/release.yml
💤 Files with no reviewable changes (4)
  • .github/workflows/integration-tests.yml
  • .github/workflows/ci.yml
  • .github/workflows/prepare-release.yml
  • .github/workflows/release.yml

Walkthrough

This 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 gh release create draft flow that attaches artifacts.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Improve GitHub Actions' is vague and generic, using non-descriptive phrasing that doesn't convey the specific changes involved in pinning actions, disabling credentials, or replacing release tools. Consider a more specific title that captures the main changes, such as 'Pin GitHub Actions and improve security practices' or 'Lock action versions and disable credential persistence'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly relates to the changeset by outlining the key objectives: pinning actions, disabling credential persistence, removing caches, preventing template expansion, and replacing the release action.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 win

Same matrix.target issue 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 win

Same matrix.target issue 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

📥 Commits

Reviewing files that changed from the base of the PR and between d03edef and 702665b.

📒 Files selected for processing (4)
  • .github/workflows/ci.yml
  • .github/workflows/integration-tests.yml
  • .github/workflows/prepare-release.yml
  • .github/workflows/release.yml

Comment thread .github/workflows/prepare-release.yml Outdated
Comment thread .github/workflows/prepare-release.yml
Comment thread .github/workflows/release.yml Outdated
@greptile-apps

greptile-apps Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

All 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

Comment thread .github/workflows/prepare-release.yml Outdated
Comment thread .github/workflows/prepare-release.yml Outdated
Comment thread .github/workflows/release.yml
@RobinMalfait RobinMalfait force-pushed the chore/improve-actions branch from 702665b to c2c22aa Compare May 21, 2026 09:29
To prevent cache poisoning attacks
The shell itself will perform variable expansion for us.
@RobinMalfait RobinMalfait force-pushed the chore/improve-actions branch from c2c22aa to e5fc24a Compare May 21, 2026 09:31
This is auto-discovered baseed on the `packageManager` field. Let's use
that as the source of truth
@RobinMalfait RobinMalfait force-pushed the chore/improve-actions branch from e5fc24a to 8fc6ebf Compare May 21, 2026 09:56
@RobinMalfait RobinMalfait merged commit 161569a into main May 21, 2026
9 checks passed
@RobinMalfait RobinMalfait deleted the chore/improve-actions branch May 21, 2026 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant