ci: harden GitHub Actions with permissions, SHA pins, and cache restrictions - #154
Conversation
- Add workflow-level permissions: contents: read - Pin all actions to SHA commits with version comments - Add save-if restrictions to rust-cache (only save on main branch)
…ictions release.yml: - Pin all actions to SHA commits with version comments - Add save-if restriction to rust-cache (only save on tag pushes) security.yml: - Pin all actions to SHA commits with version comments - Add save-if restriction to rust-cache (only save on main branch)
test-action.yml:
- Add workflow-level permissions: contents: read
- Pin all actions to SHA commits with version comments
- Add save-if restrictions to both rust-cache usages
changelog.yml:
- Add workflow-level permissions: contents: read
- Pin checkout action to SHA commit
claude.yml:
- Add workflow-level permissions: {} (jobs specify their own)
- Pin checkout and claude-code-action to SHA commits
claude-code-review.yml:
- Add workflow-level permissions: {} (job specifies its own)
- Pin checkout and claude-code-action to SHA commits
- Document security hardening approach (permissions, SHA pins, cache restrictions) - Provide SHA pin reference table for all actions used - Include instructions for updating action versions - Add workflow overview table
Summary of ChangesHello @avifenesh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the security posture of the project's continuous integration pipelines. By applying a set of best practices, it aims to reduce potential vulnerabilities related to workflow permissions, the integrity of third-party actions, and cache security, thereby making the CI/CD process more robust and trustworthy. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71d9739a79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -170,12 +175,14 @@ jobs: | |||
| security-events: write | |||
There was a problem hiding this comment.
The workflow now sets permissions: contents: read at the top level, but this job overrides it with a job-level permissions block that only includes security-events: write. Job-level permissions replace workflow defaults, so this job no longer has contents: read, which actions/checkout relies on. That means the Test SARIF Upload job will fail at checkout when the hardened permissions change takes effect. Add contents: read to this job’s permissions to keep checkout working while still limiting access.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant security hardening for the GitHub Actions workflows, and the changes are well-documented in the changelog. The new entry is clear and correctly placed under the ### Security section. I've added one comment regarding the changelog structure to improve its overall consistency and readability.
| - Restricted cache saves to main branch only (prevents cache poisoning from PRs) | ||
| - Documented SHA pin reference in .github/workflows/README.md for maintainability | ||
|
|
||
| ### Added |
There was a problem hiding this comment.
This changelog appears to have multiple ### Added sections under ## [Unreleased]. According to the 'Keep a Changelog' format, there should only be one of each heading type per release. To improve clarity and consistency, consider consolidating all new features under the first ### Added section (at line 10).
Job-level permissions override workflow defaults, so the checkout action needs explicit contents:read permission in addition to security-events:write.
There was a problem hiding this comment.
Pull request overview
This pull request implements comprehensive security hardening for all GitHub Actions workflows in the repository, addressing issue #91. The changes follow security best practices to reduce supply chain attack risks and prevent cache poisoning attacks.
Changes:
- Added explicit permissions blocks to all 7 workflows following the principle of least privilege
- SHA-pinned 10 unique third-party actions to specific commit hashes to prevent supply chain attacks
- Restricted cache saves to protected branches (main) and tag pushes to prevent cache poisoning from pull requests
- Added comprehensive documentation in
.github/workflows/README.mdto guide future maintenance
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/ci.yml | Added permissions block and SHA-pinned all actions; restricted cache saves to main branch |
| .github/workflows/changelog.yml | Added permissions block and SHA-pinned checkout action |
| .github/workflows/test-action.yml | Added permissions block, SHA-pinned actions, and restricted cache saves to main branch |
| .github/workflows/security.yml | SHA-pinned all actions and restricted cache saves to main branch |
| .github/workflows/release.yml | SHA-pinned all actions and restricted cache saves to tag pushes |
| .github/workflows/claude.yml | Added empty permissions block at workflow level and SHA-pinned actions |
| .github/workflows/claude-code-review.yml | Added empty permissions block at workflow level and SHA-pinned actions |
| .github/workflows/README.md | Added comprehensive documentation of security practices, SHA pin reference table, and update procedures |
| CHANGELOG.md | Added Security section entry documenting the workflow hardening changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - uses: Swatinem/rust-cache@v2 | ||
| - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2 | ||
| with: | ||
| save-if: ${{ github.ref == 'refs/heads/main' }} |
There was a problem hiding this comment.
The cache save restriction in test-action.yml only allows caching on the main branch, but this workflow is configured to run on feature/* and fix/* branches (see lines 5-9). This means builds on feature/* and fix/* branches will never be able to save caches, potentially slowing down development on these branches.
Consider whether these branches should also be allowed to save caches, or if the trigger pattern should be restricted. The current configuration prevents cache poisoning but may impact developer experience on feature branches.
| save-if: ${{ github.ref == 'refs/heads/main' }} | |
| save-if: ${{ github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/feature/') || startsWith(github.ref, 'refs/heads/fix/') }} |
| - Restricted cache saves to main branch only (prevents cache poisoning from PRs) | ||
| - Documented SHA pin reference in .github/workflows/README.md for maintainability | ||
|
|
||
| ### Added |
There was a problem hiding this comment.
The CHANGELOG now has duplicate "### Added" sections (one at line 10 and one at line 63). According to Keep a Changelog format, each section type should appear only once per release. The two "### Added" sections should be merged into a single section.
| - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2 | ||
| with: | ||
| key: release-${{ matrix.target }} | ||
| save-if: ${{ startsWith(github.ref, 'refs/tags/') }} |
There was a problem hiding this comment.
The release.yml workflow uses a different cache save condition than described in the PR description. The description states "Restricted cache saves to main branch only" but release.yml correctly uses startsWith(github.ref, 'refs/tags/') instead since it runs on tag pushes, not on main branch pushes. While the implementation is correct, the PR description could be more precise about this distinction.
- Merged duplicate ### Added sections into one - Removed duplicate ### Fixed section (content already present) - Now follows Keep a Changelog format with one heading per type
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -55,8 +47,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| - New `file_utils` module with `safe_read_file()` using `symlink_metadata()` | |||
| - Applied to validation, imports, fixes, and config loading | |||
| - Cross-platform tests for Unix and Windows symlink handling | |||
| - Hardened GitHub Actions workflows with security best practices: | |||
| - Added explicit permissions blocks to all workflows (principle of least privilege) | |||
| - SHA-pinned all third-party actions to prevent supply chain attacks | |||
| - Restricted cache saves to main branch only (prevents cache poisoning from PRs) | |||
| - Documented SHA pin reference in .github/workflows/README.md for maintainability | |||
|
|
|||
| ### Added | |||
| - MCP-008 rule for protocol version validation with configurable `mcp_protocol_version` option | |||
| - 5 new parse error rules with normalized IDs (AS-016, CC-HK-012, CC-AG-007, CC-PL-006, MCP-007) | |||
| - Auto-fix support for CC-MEM-005 and CC-MEM-007 memory rules | |||
| - CC-MEM-005: Delete lines containing generic instructions | |||
| - CC-MEM-007: Replace weak constraint language with stronger alternatives | |||
| - CRLF line ending support for correct byte offsets on Windows | |||
| - Auto-fix implementations for five additional rules: | |||
There was a problem hiding this comment.
The CHANGELOG structure has been reorganized in a way that moves existing entries around. Specifically, the "Added" section entries for MCP-008, parse error rules, and auto-fix support (originally at lines 10-19) have been moved to after the new Security entry (lines 56-62). Additionally, the "Fixed" section entries for exclude glob patterns and PE-001 rules (originally at lines 227-234) appear to have been removed entirely.
This reorganization may cause confusion as it modifies existing changelog entries from their original position. Keep a Changelog best practices suggest that entries should be added to their respective sections without reorganizing existing content. If these entries were meant to stay in the Unreleased section, they should remain in their original order with new entries added above them or in a consistent pattern.
Summary
Changes
Permissions Hardening
permissions: contents: readto ci.yml, changelog.yml, test-action.ymlpermissions: {}to claude.yml, claude-code-review.yml (jobs have specific permissions)SHA Pinning (10 unique actions)
Cache Hardening
save-if: ${{ github.ref == 'refs/heads/main' }}to all rust-cache usagesDocumentation
.github/workflows/README.mddocumenting the SHA pinning strategyCloses #91
Test plan