Enhance CI workflow with security and reproducibility #4
Conversation
Added minimal permissions to reduce GitHub token exposure and switched to using pnpm's frozen lockfile for reproducible dependency installs. Improved artifact upload for coverage reports and build outputs, and made formatting consistent throughout the workflow.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the CI/CD pipeline with security improvements and build reproducibility. The changes add minimal GitHub token permissions to follow the principle of least privilege, enforce frozen lockfile installs to ensure reproducible builds, and improve artifact handling for test coverage and build outputs.
- Added minimal permissions (
contents: read) at the workflow level to reduce security risks - Switched to
pnpm install --frozen-lockfileacross all jobs for reproducible dependency installations - Enhanced artifact upload for coverage reports and improved consistency in formatting
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/ci.yml
Outdated
| path: | | ||
| coverage | ||
| coverage/**/* |
There was a problem hiding this comment.
The path specification is redundant. The pattern coverage already matches the directory and coverage/**/* adds no additional coverage. Simplify to just coverage/ to capture the entire directory contents.
| path: | | |
| coverage | |
| coverage/**/* | |
| path: coverage/ |
.github/workflows/ci.yml
Outdated
| run: pnpm run test | ||
|
|
||
| - name: Upload coverage artifact (if produced) | ||
| if: always() |
There was a problem hiding this comment.
Using if: always() will attempt to upload artifacts even when the test step is skipped or cancelled, not just on failure. Consider using if: success() || failure() instead to only upload when tests actually run but potentially fail, avoiding unnecessary uploads when the workflow is cancelled or skipped.
| if: always() | |
| if: success() || failure() |
| runs-on: ubuntu-latest | ||
| needs: [build] | ||
| needs: [ build ] | ||
| if: github.event_name == 'push' && github.ref == 'refs/heads/main' |
There was a problem hiding this comment.
The docker job builds and caches Docker images but has only contents: read permission. If this job needs to use GitHub Actions cache (type=gha), it requires actions: write permission. Consider adding job-level permissions or documenting that the cache operations may fail silently with the current minimal permissions.
| if: github.event_name == 'push' && github.ref == 'refs/heads/main' | |
| if: github.event_name == 'push' && github.ref == 'refs/heads/main' | |
| permissions: | |
| contents: read | |
| actions: write |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@CorentynDevPro I've opened a new pull request, #5, to work on those changes. Once the pull request is ready, I'll request review from you. |
- Changed coverage artifact upload condition to `success() || failure()` - Simplified coverage artifact path to `coverage/` - Added `actions: write` permission to docker job Co-authored-by: CorentynDevPro <[email protected]>
Apply bot review comments: fix coverage upload condition and docker permissions
Added minimal permissions to reduce GitHub token exposure and switched to using pnpm's frozen lockfile for reproducible dependency installs. Improved artifact upload for coverage reports and build outputs, and made formatting consistent throughout the workflow.