[StepSecurity] Apply security best practices #1791
Conversation
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-university ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
❌ Deploy Preview for kleros-v2-testnet-devtools failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe changes in this pull request involve updates to a GitHub Actions workflow and a Dockerfile. In the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/contracts-testing.yml (2)
Line range hint
64-66: Clean up commented codeRemove the commented out parent dependencies installation section if it's no longer needed. If it might be needed later, add a comment explaining why it's preserved.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 26-26: wrong indentation: expected 6 but found 4
(indentation)
45-46: Fix trailing whitespaceRemove trailing whitespace from lines 45, 46, and 48 to maintain consistent formatting.
Also applies to: 48-48
🧰 Tools
🪛 yamllint (1.35.1)
[error] 45-45: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/contracts-testing.yml(3 hunks)services/bots/base/Dockerfile(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- services/bots/base/Dockerfile
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/contracts-testing.yml
[error] 45-45: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/contracts-testing.yml (4)
Line range hint 13-14: LGTM! Good security practice with minimal permissions
The addition of explicit contents: read permission follows the principle of least privilege, which is a security best practice for GitHub Actions workflows.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 26-26: wrong indentation: expected 6 but found 4
(indentation)
27-27: LGTM! Proper pinning of GitHub Actions
All GitHub Actions have been correctly pinned to specific commit hashes while maintaining version references in comments. This is a security best practice that prevents supply chain attacks through action version tampering.
Also applies to: 43-43, 47-47, 50-50, 81-81
Line range hint 27-39: LGTM! Comprehensive egress policy configuration
The harden-runner configuration includes:
- Disabled sudo access
- Blocked egress by default
- Explicit allowlist for required endpoints
This follows security best practices by implementing the principle of least privilege for network access.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 26-26: wrong indentation: expected 6 but found 4
(indentation)
Line range hint 11-12: Consider restricting PR trigger branches
The workflow currently runs on PRs from any branch (branches: ["*"]). Consider restricting this to specific branches or adding pull_request_target with proper safeguards if you need to handle external contributions.
✅ Verification successful
Consider restricting PR trigger branches and enhancing security controls
The workflow has several security considerations that need attention:
- It runs on pull requests from any branch (
branches: ["*"]), which could be a security risk - However, it has proper security controls in place:
- Uses
permissions: contents: readto limit scope - Implements Harden Runner with strict egress policies
- Has proper cache key versioning with
CACHE_VERSIONsecret - Uses pinned action versions
- Uses
Recommendations:
- Restrict PR triggers to specific trusted branches (e.g.,
branches: [master, dev]) - Keep the existing security controls as they provide good protection
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any other workflow files that might handle external PRs
rg -l 'pull_request_target' .github/workflows/
Length of output: 46
Script:
#!/bin/bash
# Check all workflow files for PR triggers and their configurations
rg -A 5 'on:.*pull' .github/workflows/
Length of output: 40
Script:
#!/bin/bash
# Let's check the workflow file content directly
cat .github/workflows/contracts-testing.yml
Length of output: 2506
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 26-26: wrong indentation: expected 6 but found 4
(indentation)
jaybuidl
added
Type: Maintenance 🚧
Type: Toolchain ⚒️
Signed-off-by: StepSecurity Bot <[email protected]>
jaybuidl
force-pushed
the
stepsecurity_remediation_1734114418
branch
from
66d0acd to
1fe55c7
Compare
|
Code Climate has analyzed commit 1fe55c7 and detected 0 issues on this pull request. View more on Code Climate. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/contracts-testing.yml (1)
43-48: Clean up trailing spaces while maintaining secure configurationThe security improvements using commit hashes are excellent. However, there are some minor formatting issues to clean up.
- node-version: 18.x + node-version: 18.x🧰 Tools
🪛 yamllint (1.35.1)
[error] 45-45: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/contracts-testing.yml(3 hunks)services/bots/base/Dockerfile(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- services/bots/base/Dockerfile
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/contracts-testing.yml
[error] 45-45: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.github/workflows/contracts-testing.yml (5)
Line range hint 1-26: LGTM! Well-structured security enhancements
The changes follow security best practices by:
- Explicitly declaring minimal required permissions (contents: read)
- Properly scoping workflow triggers with paths-ignore
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 26-26: wrong indentation: expected 6 but found 4
(indentation)
Line range hint 27-42: Excellent security hardening configuration!
The Harden Runner step is well configured with:
- Pinned action to specific commit hash
- Strict egress policy blocking all traffic by default
- Explicit allowlist of required endpoints
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 26-26: wrong indentation: expected 6 but found 4
(indentation)
Line range hint 49-60: LGTM! Secure caching configuration
The cache configuration is well-structured with:
- Pinned action to specific commit hash
- Proper cache key strategy using environment secret
- Appropriate restore-keys fallback
🧰 Tools
🪛 yamllint (1.35.1)
[error] 45-45: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
81-84: LGTM! Secure artifact upload configuration
The upload-artifact action has been properly secured by pinning to a specific commit hash while maintaining the existing functionality.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 82-82: trailing spaces
(trailing-spaces)
[error] 83-83: trailing spaces
(trailing-spaces)
Line range hint 1-84: Verify action commit hashes match stated versions
While the security improvements are excellent, let's verify that the commit hashes correspond to their stated versions.
✅ Verification successful
Action commit hashes match their stated versions
Based on the GitHub API responses, all actions are using valid releases that match their stated versions:
- step-security/[email protected] (released Sep 2024)
- actions/[email protected] (released Oct 2024)
- actions/[email protected] (released Oct 2024)
- actions/[email protected] (released Dec 2024)
- actions/[email protected] (released Oct 2024)
All actions are using recent, stable releases with proper version pinning through commit hashes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the commit hashes match their stated versions
# For each action, we'll check the release tag associated with the commit
# Function to check commit against release
check_commit() {
local owner=$1
local repo=$2
local commit=$3
local version=$4
echo "Checking $owner/$repo@$commit for version $version"
gh api repos/$owner/$repo/commits/$commit/tags
}
# Check each action
check_commit "step-security" "harden-runner" "91182cccc01eb5e619899d80e4e971d6181294a7" "v2.10.1"
check_commit "actions" "setup-node" "39370e3970a6d050c480ffad4ff0ed4d3fdee5af" "v4.1.0"
check_commit "actions" "checkout" "11bd71901bbe5b1630ceea73d27597364c9af683" "v4.2.2"
check_commit "actions" "cache" "1bd1e32a3bdc45362d1e726936510720a7c30a57" "v4.2.0"
check_commit "actions" "upload-artifact" "b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882" "v4.4.3"
Length of output: 3763
Script:
#!/bin/bash
# Let's try a different approach using the GitHub API to get the release information directly
check_release() {
local owner=$1
local repo=$2
local version=$3
echo "Checking $owner/$repo release $version"
gh api repos/$owner/$repo/releases/tags/$version
}
# Check each action's release
check_release "step-security" "harden-runner" "v2.10.1"
check_release "actions" "setup-node" "v4.1.0"
check_release "actions" "checkout" "v4.2.2"
check_release "actions" "cache" "v4.2.0"
check_release "actions" "upload-artifact" "v4.4.3"
Length of output: 13885
🧰 Tools
🪛 yamllint (1.35.1)
[error] 45-45: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
Summary
This pull request is created by StepSecurity at the request of @jaybuidl. Please merge the Pull Request to incorporate the requested changes. Please tag @jaybuidl on your message if you have any questions related to the PR.
Security Fixes
Pinned Dependencies
GitHub Action tags and Docker tags are mutable. This poses a security risk. GitHub's Security Hardening guide recommends pinning actions to full length commit.
Secure Dockerfiles
Pin image tags to digests in Dockerfiles. With the Docker v2 API release, it became possible to use digests in place of tags when pulling images or to use them in FROM lines in Dockerfiles.
Feedback
For bug reports, feature requests, and general feedback; please email [email protected]. To create such PRs, please visit https://app.stepsecurity.io/securerepo.
Signed-off-by: StepSecurity Bot [email protected]
PR-Codex overview
This PR updates the base
Dockerfileand modifies the GitHub Actions workflow by changing the versions of several actions used in the CI/CD process, enhancing security and compatibility.Detailed summary
FROMstatement inservices/bots/base/Dockerfileto usenode:20-alpine.harden-runnerversion to91182cccc01eb5e619899d80e4e971d6181294a7.setup-nodeaction to39370e3970a6d050c480ffad4ff0ed4d3fdee5af.checkoutaction to11bd71901bbe5b1630ceea73d27597364c9af683.cacheaction version to1bd1e32a3bdc45362d1e726936510720a7c30a57.upload-artifactaction tob4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882.Summary by CodeRabbit