Skip to content

ci: tighten GitHub Actions workflow security#1328

Open
emptyhammond wants to merge 7 commits into
mainfrom
worktree-fixup-workflows
Open

ci: tighten GitHub Actions workflow security#1328
emptyhammond wants to merge 7 commits into
mainfrom
worktree-fixup-workflows

Conversation

@emptyhammond

@emptyhammond emptyhammond commented May 26, 2026

Copy link
Copy Markdown

Routine hardening of the workflows under .github/workflows/ based on a zizmor audit. No CI behaviour changes are intended - pinned action versions match what the floating tags currently resolve to, and the added permissions match what the workflows already do.

Summary by CodeRabbit

  • Chores
    • Pinned GitHub Actions steps to specific commit versions across CI workflows for more consistent builds.
    • Strengthened CI security by setting explicit minimal GitHub token permissions and disabling credential persistence in checkout steps.
    • Updated packaging/build workflows to use explicit permissions and forwarding of required secrets for safer, more controlled deployments.

Move `github.event.inputs.version` from inline expressions into step `env:`
blocks so the value is interpolated by the shell rather than substituted
into the shell command at workflow-rendering time. Addresses zizmor
`template-injection` findings in the three packaging steps.
Pin all `uses:` references to immutable commit hashes (with the
resolved tag as a trailing comment), so a compromise of an upstream
tag cannot silently change what runs in CI. The pinned versions
match what the floating tags currently resolve to, so no behaviour
change is intended. Addresses zizmor `unpinned-uses` findings.
Set `persist-credentials: false` on every `actions/checkout` step so the
GITHUB_TOKEN is not left available to subsequent steps after the
checkout completes. None of the workflows push back to the repo, so
no compensating change is needed. Addresses zizmor `artipacked`
findings.
Add a top-level `permissions: contents: read` block to every workflow
so the GITHUB_TOKEN is scoped down from the repo's default rather
than relying on whatever the default happens to be. None of these
workflows need to write back to the repo (no commits, comments,
releases or package publishing happens here), so read-only is enough.
Addresses zizmor `excessive-permissions` findings.
The reusable `ably/features/.github/workflows/sdk-features.yml` workflow
declares one required secret (`ABLY_AWS_ACCOUNT_ID_SDK`) plus the
auto-provided GITHUB_TOKEN, so there is no reason to forward every
secret on this repo to it. Replace `secrets: inherit` with an
explicit map containing just that secret. Addresses zizmor
`secrets-inherit` finding.
Revert the SHA pin on `ably/features/.github/workflows/sdk-features.yml`
back to `@main`. This workflow is the SDK feature-spec compliance
suite — pinning it freezes the spec under test, which defeats the
purpose of running it on every PR. The repo is org-owned and trusted,
so the `unpinned-uses` finding is suppressed inline rather than
fixed.
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8fba457d-f98c-487c-9bb4-45c3bb8cd5f0

📥 Commits

Reviewing files that changed from the base of the PR and between 303b594 and bea3f78.

📒 Files selected for processing (1)
  • .github/workflows/features.yml

Walkthrough

Seven GitHub Actions workflows update permissions, pin action versions, disable checkout credential persistence, explicitly pass one reusable-workflow secret, and route package script version inputs through an environment variable.

Changes

GitHub Actions Security Hardening

Layer / File(s) Summary
Restrict workflow permissions
.github/workflows/features.yml, .github/workflows/package.yml, .github/workflows/run-tests-linux.yml, .github/workflows/run-tests-macos-mono.yml, .github/workflows/run-tests-macos.yml, .github/workflows/run-tests-windows-netframework.yml, .github/workflows/run-tests-windows.yml
Workflows declare minimal GitHub token permissions, with the features workflow using an empty top-level permissions block and the other workflows limiting access to contents: read.
Pin checkout and setup actions
.github/workflows/package.yml, .github/workflows/run-tests-linux.yml, .github/workflows/run-tests-macos-mono.yml, .github/workflows/run-tests-macos.yml, .github/workflows/run-tests-windows-netframework.yml, .github/workflows/run-tests-windows.yml
Checkout, setup-dotnet, and related package actions move from floating tags to pinned revisions, and checkout steps disable credential persistence.
Pass build secrets explicitly
.github/workflows/features.yml
The build job keeps the same reusable workflow call but replaces secrets: inherit with an explicit mapping for ABLY_AWS_ACCOUNT_ID_SDK.
Use version env vars in package jobs
.github/workflows/package.yml
Package, package-push, and package-unity jobs pass the workflow version through env.VERSION, invoke their scripts with that variable, and pin artifact upload/download actions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hoppity hop, the workflows gleam,
Pinned SHAs now steady the stream.
Read-only paws and secrets just right,
Versioned script calls in the moonlit light.
The bunny approves this tidy delight.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: hardening GitHub Actions workflows and permissions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-fixup-workflows

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.

❤️ Share

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

@emptyhammond emptyhammond requested a review from VeskeR May 26, 2026 10:28

@VeskeR VeskeR left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Need to fix permissions sdk-features here and in other affected SDKs

build:
uses: ably/features/.github/workflows/sdk-features.yml@main
# Track the live SDK feature spec on main rather than pinning.
uses: ably/features/.github/workflows/sdk-features.yml@main # zizmor: ignore[unpinned-uses]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

sdk-features workflow requires permissions:

      deployments: write
      id-token: write

, but top level configured to only contents: read. This results in an invisible failure during workflow startup, and it's currently broken, see: https://github.com/ably/ably-dotnet/actions/runs/26446846130.

I can see the features workflow is now also broken for at least ably-js and ably-python since the related github actions hardening PRs have landed: ably-js, ably-python.
More SDK repos might be affected. Happy to review the fixes for them too.

The reusable ably/features sdk-features.yml job needs deployments:write
and id-token:write (OIDC AWS auth + deployment upload). The top-level
contents:read added during workflow hardening stripped these, causing an
invisible startup failure of the Features workflow.

Grant the required scopes at the build job level (least privilege) and
default the top level to no permissions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants