ci: harden GitHub Actions with permissions, SHA pins, and cache restrictions #154
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| # GitHub Actions Workflows | ||
|
|
||
| This directory contains CI/CD workflows for the agnix project. | ||
|
|
||
| ## Security Hardening | ||
|
|
||
| All workflows follow security best practices: | ||
|
|
||
| ### 1. Explicit Permissions | ||
|
|
||
| Every workflow declares minimum required permissions at the workflow level. | ||
| Jobs that need additional permissions declare them at the job level. | ||
|
|
||
| - `permissions: {}` - No permissions (used when jobs specify their own) | ||
| - `permissions: contents: read` - Read-only access to repository contents | ||
|
|
||
| ### 2. SHA-Pinned Actions | ||
|
|
||
| All third-party actions are pinned to specific commit SHAs to prevent | ||
| supply chain attacks. The SHA pins are documented with version comments | ||
| for maintainability. | ||
|
|
||
| ### 3. Cache Save Restrictions | ||
|
|
||
| Rust caches (`Swatinem/rust-cache`) are configured with `save-if` conditions | ||
| to only save caches on protected branches (main) or tag pushes. This prevents | ||
| cache poisoning from pull requests. | ||
|
|
||
| ## SHA Pin Reference | ||
|
|
||
| When updating actions, use these SHA commits (last verified: 2025-02): | ||
|
|
||
| ```yaml | ||
| # GitHub Official Actions | ||
| actions/checkout@v4: 34e114876b0b11c390a56381ad16ebd13914f8d5 | ||
| actions/upload-artifact@v4: ea165f8d65b6e75b540449e92b4886f43607fa02 | ||
| actions/download-artifact@v4: d3f86a106a0bac45b974a628896c90dbdf5c8093 | ||
|
|
||
| # Rust Tooling | ||
| dtolnay/rust-toolchain@stable: 4be9e76fd7c4901c61fb841f559994984270fce7 | ||
| Swatinem/rust-cache@v2: 779680da715d629ac1d338a641029a2f4372abb5 | ||
| taiki-e/install-action@v2: 650c5ca14212efbbf3e580844b04bdccf68dac31 | ||
| taiki-e/install-action@nextest: cd05dcd6eb73067dda063b97a15b7060049dacd9 | ||
|
|
||
| # Security | ||
| github/codeql-action@v3: 2588666de8825e1e9dc4e2329a4c985457d55b32 | ||
|
|
||
| # Release | ||
| softprops/action-gh-release@v2: a06a81a03ee405af7f2048a818ed3f03bbf83c7b | ||
|
|
||
| # Claude Code | ||
| anthropics/claude-code-action@v1: 6867bb3ab0b2c0a10629b6823e457347e74ad6d2 | ||
| ``` | ||
|
|
||
| ## Updating Action Versions | ||
|
|
||
| When a new version of an action is released: | ||
|
|
||
| 1. Check the release notes for security implications | ||
| 2. Get the full SHA of the release tag: | ||
| ```bash | ||
| git ls-remote --tags https://github.com/owner/repo refs/tags/vX.Y.Z | ||
| ``` | ||
| 3. Update all occurrences in workflow files | ||
| 4. Update this README with the new SHA | ||
| 5. Test the workflows on a feature branch before merging | ||
|
|
||
| ## Workflow Overview | ||
|
|
||
| | Workflow | Trigger | Purpose | | ||
| |----------|---------|---------| | ||
| | ci.yml | push/PR to main | Lint, test, build | | ||
| | release.yml | tag push (v*) | Build and publish releases | | ||
| | security.yml | push/PR/schedule | CodeQL analysis and security audit | | ||
| | test-action.yml | push/PR (action paths) | Test the GitHub Action | | ||
| | changelog.yml | PR | Verify CHANGELOG.md is updated | | ||
| | claude.yml | issue/PR comments | Claude Code assistant | | ||
| | claude-code-review.yml | PR | Automated code review | |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -14,6 +14,9 @@ on: | |||||
| - '.github/workflows/test-action.yml' | ||||||
| workflow_dispatch: | ||||||
|
|
||||||
| permissions: | ||||||
| contents: read | ||||||
|
|
||||||
| jobs: | ||||||
| test-action: | ||||||
| name: Test (${{ matrix.os }}) | ||||||
|
|
@@ -24,12 +27,14 @@ jobs: | |||||
| os: [ubuntu-latest, macos-latest, windows-latest] | ||||||
|
|
||||||
| steps: | ||||||
| - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 | ||||||
|
|
||||||
| - name: Install Rust | ||||||
| uses: dtolnay/rust-toolchain@4be9e76fd7c4901c61fb841f559994984270fce7 # stable | ||||||
|
|
||||||
| - 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.
Suggested change
|
||||||
|
|
||||||
| - name: Test action (default inputs) | ||||||
| id: test-default | ||||||
|
|
@@ -170,12 +175,14 @@ jobs: | |||||
| security-events: write | ||||||
|
There was a problem hiding this comment.
The workflow now sets Useful? React with 👍 / 👎. |
||||||
|
|
||||||
| steps: | ||||||
| - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 | ||||||
|
|
||||||
| - name: Install Rust | ||||||
| uses: dtolnay/rust-toolchain@4be9e76fd7c4901c61fb841f559994984270fce7 # stable | ||||||
|
|
||||||
| - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2 | ||||||
| with: | ||||||
| save-if: ${{ github.ref == 'refs/heads/main' }} | ||||||
|
|
||||||
| - name: Run agnix with SARIF | ||||||
| id: agnix | ||||||
|
|
@@ -187,7 +194,7 @@ jobs: | |||||
|
|
||||||
| - name: Upload SARIF to GitHub | ||||||
| if: steps.agnix.outputs.sarif-file != '' | ||||||
| uses: github/codeql-action/upload-sarif@2588666de8825e1e9dc4e2329a4c985457d55b32 # v3 | ||||||
| with: | ||||||
| sarif_file: ${{ steps.agnix.outputs.sarif-file }} | ||||||
| continue-on-error: true | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,11 @@ | |
| - 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 | ||
|
Contributor
There was a problem hiding this comment. This changelog appears to have multiple 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. |
||
| - Auto-fix implementations for five additional rules: | ||
|
Comment on lines
10
to
63
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. |
||
|
|
@@ -134,7 +139,7 @@ | |
| - Cross-platform validation rules XP-001, XP-002, XP-003 | ||
| - XP-001: Detects Claude-specific features (hooks, context:fork, agent, allowed-tools) in AGENTS.md (error) | ||
| - XP-002: Validates AGENTS.md markdown structure for cross-platform compatibility (warning) | ||
| - XP-003: Detects hard-coded platform paths (.claude/, .opencode/, .cursor/, etc.) in configs (warning) | ||
|
Check warning on line 142 in CHANGELOG.md
|
||
| - New `cross_platform` config category toggle for XP-* rules | ||
| - 5 test fixtures in tests/fixtures/cross_platform/ directory | ||
| - 30 comprehensive unit tests for cross-platform validation | ||
|
|
@@ -203,7 +208,7 @@ | |
| - CC-AG-002: Validates required 'description' field in agent frontmatter | ||
| - CC-AG-003: Validates model values (sonnet, opus, haiku, inherit) | ||
| - CC-AG-004: Validates permissionMode values (default, acceptEdits, dontAsk, bypassPermissions, plan) | ||
| - CC-AG-005: Validates referenced skills exist at .claude/skills/[name]/SKILL.md | ||
|
Check warning on line 211 in CHANGELOG.md
|
||
| - CC-AG-006: Detects conflicts between 'tools' and 'disallowedTools' arrays | ||
| - Path traversal security protection for skill name validation | ||
| - 44 comprehensive tests for agent validation (152 total tests) | ||
|
|
@@ -213,7 +218,7 @@ | |
| - Comprehensive tests for parallel validation edge cases | ||
| - Reference validator rules REF-001 and REF-002 | ||
| - REF-001: @import references must point to existing files (error) | ||
| - REF-002: Markdown links [text](path) should point to existing files (error) | ||
|
Check failure on line 221 in CHANGELOG.md
|
||
| - Both rules are in the "imports" category | ||
| - Supports fragment stripping (file.md#section validates file.md) | ||
| - Skips external URLs (http://, https://, mailto:, etc.) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.