ci: harden workflow security#1211
Open
emptyhammond wants to merge 7 commits into
Open
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughGitHub Actions across seven workflow files are pinned to specific commit SHAs instead of floating version tags. Integration test workflows additionally gain job-level permissions and disable credential persistence in checkout steps. The features workflow replaces inherited secrets with explicit secret mapping while being pinned to a commit. ChangesGitHub Actions Dependency Pinning
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
emptyhammond
changed the title
Pin every `uses:` reference in the 7 workflow files to a 40-character commit SHA, preserving the original tag as a trailing comment. Versions are not changed — each action is pinned to the commit that the previously-floating tag resolved to at the time of this commit. This mitigates supply-chain risk from compromised or moved tags: a malicious actor able to force-push a tag in a third-party action repo can no longer affect this repository's CI without first compromising a specific SHA that has already been vetted here. Includes the reusable workflow reference in features.yml (`ably/features/.github/workflows/sdk-features.yml`), which was previously tracking `@main`.
Add an explicit `permissions: contents: read` block to each of the 5 jobs in the Integration Test workflow. Previously the workflow relied on the repository default token permissions, which grant broader write scopes than these jobs need. These jobs only check out source (read), build with Gradle, and (for two of them) upload artifacts via `actions/upload-artifact`. The upload uses the runtime artifacts API rather than a `GITHUB_TOKEN` scope, so `contents: read` is sufficient for all 5 jobs. Scoped per-job rather than workflow-wide so each job declares its own minimum permissions and a future job added here cannot silently inherit a broader default.
Add `persist-credentials: false` to each of the 5 `actions/checkout` steps in the Integration Test workflow. By default `actions/checkout` writes a short-lived `GITHUB_TOKEN`-derived credential into the local `.git/config` for the duration of the job. That credential can then leak into uploaded artifacts, into subprocesses that read the git config, or into remote-tracking operations subsequent steps perform. None of the integration test jobs push, tag, or otherwise need write access to the repository over git — they only need a working tree to build and test against — so the persisted credential is pure attack surface.
Add an explicit `permissions: contents: read` block to the `run-on-release` job in the Maven Central publish workflow. This is the sole automated production path that publishes ably-java artifacts to Sonatype / Maven Central, so it warrants the strictest permissions footprint we can give it. Authentication to Maven Central is entirely via repository secrets passed as Gradle environment variables (`SONATYPE_USERNAME`, `SONATYPE_PASSWORD`) plus the in-memory GPG signing key (`SIGNING_IN_MEMORY_KEY`, `SIGNING_KEY_ID`, `SIGNING_PASSWORD`). None of those depend on `GITHUB_TOKEN` scopes. The job performs no git push, no tag creation, no release publishing, and no other API call against the GitHub side, so `contents: read` covers everything the job actually does.
Replace `secrets: inherit` with an explicit `secrets:` map that forwards only `ABLY_AWS_ACCOUNT_ID_SDK` to the called reusable workflow. With `secrets: inherit` every secret available to this repository is implicitly handed to `ably/features/.github/workflows/sdk-features.yml` on each invocation, including secrets that workflow does not need (Sonatype publishing credentials, GPG signing keys, etc.). The called workflow declares only one required secret on its `workflow_call` trigger and otherwise relies on `GITHUB_TOKEN`, which reusable workflows receive automatically from the caller and so does not need to be passed via `secrets:`.
emptyhammond
force-pushed
the
worktree-fixup-workflows
branch
from
a32e0b2 to
d33179f
Compare
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/check.yml:
- Line 14: The checkout step currently uses the pinned action reference
"actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744" but does not disable
credential persistence; update that checkout step to include a with block
setting persist-credentials: false so the runner does not retain the
GITHUB_TOKEN in local git configuration (add the with: persist-credentials:
false parameter under the actions/checkout step).
In @.github/workflows/emulate.yml:
- Line 19: The checkout step using "uses:
actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5" leaves GITHUB_TOKEN
persisted; update that step to include a with block setting persist-credentials:
false (e.g., add "with: persist-credentials: false") so credentials are not kept
for subsequent steps that only need the repository contents.
In @.github/workflows/javadoc.yml:
- Line 16: The checkout step using
actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 needs to disable
persisted credentials for consistency with the repository hardening pattern;
update that step to include the input persist-credentials: false on the
actions/checkout invocation so the workflow does not persist GITHUB_TOKEN
credentials after checkout.
In @.github/workflows/release.yaml:
- Line 14: The release workflow's actions/checkout step is missing the
configuration to disable credential persistence; update the actions/checkout
invocation in .github/workflows/release.yaml (the uses: actions/checkout@...
entry) to include a with: block that sets persist-credentials: false so the
checkout does not leave GITHUB_TOKEN credentials available to subsequent steps.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08e4379a-c745-48cb-8d59-71ddd18d2bcf
📒 Files selected for processing (7)
.github/workflows/check.yml.github/workflows/emulate.yml.github/workflows/example-app.yml.github/workflows/features.yml.github/workflows/integration-test.yml.github/workflows/javadoc.yml.github/workflows/release.yaml
Add `persist-credentials: false` to the `actions/checkout` steps in the 5 workflows that were not covered by the earlier integration-test change: `check.yml`, `emulate.yml`, `example-app.yml`, `javadoc.yml`, and `release.yaml`. By default `actions/checkout` writes a short-lived `GITHUB_TOKEN`-derived credential into the local `.git/config` for the duration of the job. None of these jobs perform a `git push`, create tags, open PRs, or otherwise require write access to the repository over git: - `check.yml` runs Gradle linters and unit tests - `emulate.yml` runs Android emulator tests and uploads a reports artifact via `actions/upload-artifact` (which uses the runtime artifacts API, not git) - `example-app.yml` runs the example-app connectedAndroidTest - `javadoc.yml` builds Javadoc and uploads it to S3 via `ably/sdk-upload-action`; `githubToken` is passed to that action as an explicit input parameter, not via the checkout-persisted credential, so disabling persistence does not affect the upload - `release.yaml` publishes to Maven Central using Sonatype + GPG credentials from repository secrets and performs no git write `features.yml` only invokes a reusable workflow and has no checkout step of its own, so it requires no change here.
Add explicit per-job `permissions:` blocks to the 4 workflows that
were still relying on the repository default token permissions:
`check.yml`, `emulate.yml`, `example-app.yml`, `features.yml`.
- `check.yml`, `emulate.yml`, `example-app.yml` each have a single
job that only needs to read source and run Gradle. They get
`permissions: contents: read`.
- `features.yml` invokes a reusable workflow at
`ably/features/.github/workflows/sdk-features.yml`. Permissions for
a reusable workflow are inherited from the calling job — the called
workflow's own `permissions:` block cannot upgrade scopes the
caller has not granted. The called workflow at the pinned SHA runs
`actions/checkout`, then `aws-actions/configure-aws-credentials`
(AWS OIDC), then `ably/sdk-upload-action` (creates a GitHub
deployment). It therefore needs:
permissions:
contents: read
id-token: write
deployments: write
These match the inline equivalent in `javadoc.yml`, which does the
same upload work directly; `contents: read` is added here as an
explicit tightening rather than relying on the public-repo default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Address findings across the 7 GitHub Actions workflow files. Five commits, one per finding category — each commit message names the audit and the count it closes.
ci: pin GitHub Actions to commit SHAs across all workflows— pins everyuses:reference to a 40-character commit SHA with the original tag preserved as a comment. No versions changed. Includes theably/features/.github/workflows/sdk-features.ymlreusable-workflow reference, previously tracking@main. Closes 38unpinned-uses.ci(integration-test): scope GITHUB_TOKEN to contents:read per job— adds per-jobpermissions: contents: readto the 5 integration-test jobs. Closes 6excessive-permissions.ci(integration-test): disable credential persistence on checkout— addspersist-credentials: falseto the 5actions/checkoutsteps; none of these jobs push, tag, or otherwise need write access to the repository over git. Closes 5artipacked.ci(release): scope publish job permissions to contents:read— locks down the Maven Central publish job, which authenticates entirely via Sonatype + GPG repository secrets and never usesGITHUB_TOKENwrite scopes. Closes 1excessive-permissions.ci(features): pass only the secrets the called workflow actually needs— replacessecrets: inheritwith an explicit single-entrysecrets:map (ABLY_AWS_ACCOUNT_ID_SDK), the only secret the called reusable workflow declares.GITHUB_TOKENis forwarded automatically and does not need to be listed. Closes 1secrets-inherit.Summary by CodeRabbit