chore: Adding step for install #37276
Conversation
WalkthroughThe pull request introduces enhancements to the vulnerability scanning process within GitHub Actions. It modifies the workflow file to include a new step for checking vulnerabilities after running the Scout and Trivy scripts. Additionally, the scripts for Scout and Trivy have been updated to improve error handling, installation procedures, and data processing, ensuring that any detected vulnerabilities are properly logged and managed. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 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
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (7)
.github/workflows/test-vulnerabilities-data.yml (2)
Line range hint
64-80: Enhance error handling and output formatting in vulnerability check step.The vulnerability check implementation is good, but could be more robust with these improvements:
- name: Check for new vulnerabilities in Scout and Trivy files if: always() run: | + # Function to check vulnerability file + check_vulnerabilities() { + local file=$1 + local tool=$2 + + if [ ! -f "$file" ]; then + echo "::warning ::$tool vulnerability file not found" + return 0 + fi + + if [ -s "$file" ]; then + echo "::error ::$tool vulnerabilities detected:" + echo "----------------------------------------" + cat "$file" + echo "----------------------------------------" + return 1 + fi + } + - # Check if Scout vulnerabilities file is not empty - if [ -s "scout_new_vulnerabilities.csv" ]; then - echo "Scout vulnerabilities detected." - cat scout_new_vulnerabilities.csv - exit 1 # Fail the job if data exists - fi - - # Check if Trivy vulnerabilities file is not empty - if [ -s "trivy_new_vulnerabilities.csv" ]; then - echo "Trivy vulnerabilities detected." - cat trivy_new_vulnerabilities.csv - exit 1 # Fail the job if data exists - fi + # Check both vulnerability files + check_vulnerabilities "scout_new_vulnerabilities.csv" "Scout" || exit 1 + check_vulnerabilities "trivy_new_vulnerabilities.csv" "Trivy" || exit 1🧰 Tools
🪛 yamllint
[error] 74-74: trailing spaces
(trailing-spaces)
64-64: Remove trailing spaces.There are trailing spaces on lines 64 and 74 that should be removed for consistent code style.
- + - +Also applies to: 74-74
🧰 Tools
🪛 yamllint
[error] 64-64: trailing spaces
(trailing-spaces)
scripts/trivy_vulnerabilities_data.sh (5)
Line range hint
4-11: Consider enhancing credential security.While the environment variable validation is thorough, consider implementing additional security measures for database credentials:
- Use a secrets management service
- Implement credential rotation
- Add input sanitization for database parameters
Line range hint
20-47: Add version pinning and checksum verification.The Trivy installation function needs security enhancements:
TRIVY_VERSION=$(curl -s https://api.github.com/repos/aquasecurity/trivy/releases/latest | grep '"tag_name"' | sed -E 's/.*"v([^"]+)".*/\1/') +TRIVY_CHECKSUM_URL="https://github.com/aquasecurity/trivy/releases/download/v$TRIVY_VERSION/trivy_${TRIVY_VERSION}_Linux-64bit.tar.gz.sha256" +EXPECTED_CHECKSUM=$(curl -sL "$TRIVY_CHECKSUM_URL") TRIVY_URL="https://github.com/aquasecurity/trivy/releases/download/v$TRIVY_VERSION/trivy_${TRIVY_VERSION}_Linux-64bit.tar.gz" -curl -sfL "$TRIVY_URL" | tar -xzf - trivy +curl -sfLO "$TRIVY_URL" +echo "$EXPECTED_CHECKSUM trivy_${TRIVY_VERSION}_Linux-64bit.tar.gz" | sha256sum -c +tar -xzf "trivy_${TRIVY_VERSION}_Linux-64bit.tar.gz" trivy
Line range hint
77-89: Add scan timeout and externalize configuration.The Trivy scan configuration could be improved:
+# Add timeout to prevent hanging +SCAN_TIMEOUT=300 + # Run Trivy scan echo "Running Trivy scan for image: $IMAGE..." -trivy image --db-repository public.ecr.aws/aquasecurity/trivy-db --java-db-repository public.ecr.aws/aquasecurity/trivy-java-db --insecure --format json "$IMAGE" > "trivy_vulnerabilities.json" || { +timeout $SCAN_TIMEOUT trivy image \ + --db-repository "${TRIVY_DB_REPO:-public.ecr.aws/aquasecurity/trivy-db}" \ + --java-db-repository "${TRIVY_JAVA_DB_REPO:-public.ecr.aws/aquasecurity/trivy-java-db}" \ + --insecure \ + --format json \ + "$IMAGE" > "trivy_vulnerabilities.json" || { echo "Error: Trivy scan failed for image: $IMAGE" exit 1 }
Line range hint
92-146: Enhance database operations reliability.The database operations need improvements:
- Add batch processing for better performance
- Implement connection pooling
- Add retry logic for database operations
- Add proper error handling for database connection failures
Would you like me to provide a detailed implementation for these improvements?
156-156: Improve cleanup operations.Add comprehensive cleanup with error handling:
-rm -f "trivy_vulnerabilities.json" +cleanup() { + local exit_code=$? + rm -f "trivy_vulnerabilities.json" "$NEW_VULN_FILE" || true + exit $exit_code +} +trap cleanup EXIT
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
.github/workflows/test-vulnerabilities-data.yml(2 hunks)scripts/scout_vulnerabilities_data.sh(2 hunks)scripts/trivy_vulnerabilities_data.sh(1 hunks)
🧰 Additional context used
🪛 yamllint
.github/workflows/test-vulnerabilities-data.yml
[error] 64-64: trailing spaces
(trailing-spaces)
[error] 74-74: trailing spaces
(trailing-spaces)
| if ! systemctl is-active --quiet docker; then | ||
| echo "Starting Docker..." | ||
| sudo systemctl start docker | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using 'sudo' within scripts; handle Docker daemon status appropriately
Using sudo may prompt for passwords and is not recommended in scripts. Instead, check if the Docker daemon is running and inform the user if it isn't.
Apply this diff:
if ! systemctl is-active --quiet docker; then
echo "Docker is not running. Please start Docker and try again."
exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ! systemctl is-active --quiet docker; then | |
| echo "Starting Docker..." | |
| sudo systemctl start docker | |
| fi | |
| if ! systemctl is-active --quiet docker; then | |
| echo "Docker is not running. Please start Docker and try again." | |
| exit 1 | |
| fi |
| [ -s "$CSV_OUTPUT_FILE" ] || echo "No vulnerabilities found for image: $IMAGE" > "$CSV_OUTPUT_FILE" | ||
|
|
There was a problem hiding this comment.
Avoid writing messages into CSV output files
Writing non-CSV messages into the CSV file can break downstream processing. Handle the case separately without altering the CSV file.
Apply this diff:
# Check if the CSV output file is empty
-[ -s "$CSV_OUTPUT_FILE" ] || echo "No vulnerabilities found for image: $IMAGE" > "$CSV_OUTPUT_FILE"
+if [ ! -s "$CSV_OUTPUT_FILE" ]; then
+ echo "No vulnerabilities found for image: $IMAGE"
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [ -s "$CSV_OUTPUT_FILE" ] || echo "No vulnerabilities found for image: $IMAGE" > "$CSV_OUTPUT_FILE" | |
| if [ ! -s "$CSV_OUTPUT_FILE" ]; then | |
| echo "No vulnerabilities found for image: $IMAGE" | |
| fi | |
| curl -fsSL https://raw.githubusercontent.com/docker/scout-cli/main/install.sh -o install-scout.sh | ||
| sh install-scout.sh &> install_scout_log.txt |
There was a problem hiding this comment.
Verify 'curl' command success before executing the installation script
Ensure that the 'curl' command successfully downloads the installation script before executing it to prevent errors from incomplete downloads.
Apply this diff to add an error check:
curl -fsSL https://raw.githubusercontent.com/docker/scout-cli/main/install.sh -o install-scout.sh
+if [ $? -ne 0 ]; then
+ echo "Failed to download Docker Scout installation script."
+ exit 1
+fi
sh install-scout.sh &> install_scout_log.txt📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| curl -fsSL https://raw.githubusercontent.com/docker/scout-cli/main/install.sh -o install-scout.sh | |
| sh install-scout.sh &> install_scout_log.txt | |
| curl -fsSL https://raw.githubusercontent.com/docker/scout-cli/main/install.sh -o install-scout.sh | |
| if [ $? -ne 0 ]; then | |
| echo "Failed to download Docker Scout installation script." | |
| exit 1 | |
| fi | |
| sh install-scout.sh &> install_scout_log.txt |
| docker scout cves "$IMAGE" | grep -E "✗ |CVE-" | awk -v product_name="$product_name" -F' ' ' | ||
| { | ||
| # Check for valid vulnerability data and format it correctly | ||
| if ($2 != "" && $3 ~ /^CVE-/) { | ||
| # Extract severity level, CVE ID, and format output correctly | ||
| print $3","product_name",""SCOUT"","$2 | ||
| } | ||
| }' | sort -u > "$CSV_OUTPUT_FILE" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use structured output for reliable parsing of vulnerabilities
Parsing command-line output is fragile. If available, use JSON output from docker scout cves for more reliable data extraction.
Example adjustment:
-docker scout cves "$IMAGE" | grep -E "✗ |CVE-" | awk -v product_name="$product_name" -F' ' '
-{
- if ($2 != "" && $3 ~ /^CVE-/) {
- print $3","product_name",""SCOUT"","$2
- }
-}' | sort -u > "$CSV_OUTPUT_FILE"
+docker scout cves "$IMAGE" --output json | jq -r '.vulnerabilities[] | "\(.identifiers.CVE[0]),'"$product_name"',SCOUT,\(.severity)"' | sort -u > "$CSV_OUTPUT_FILE"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| docker scout cves "$IMAGE" | grep -E "✗ |CVE-" | awk -v product_name="$product_name" -F' ' ' | |
| { | |
| # Check for valid vulnerability data and format it correctly | |
| if ($2 != "" && $3 ~ /^CVE-/) { | |
| # Extract severity level, CVE ID, and format output correctly | |
| print $3","product_name",""SCOUT"","$2 | |
| } | |
| }' | sort -u > "$CSV_OUTPUT_FILE" | |
| docker scout cves "$IMAGE" --output json | jq -r '.vulnerabilities[] | "\(.identifiers.CVE[0]),'"$product_name"',SCOUT,\(.severity)"' | sort -u > "$CSV_OUTPUT_FILE" |
Description
Adding installation step.
Fixes #
37036Automation
/ok-to-test tags=""
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Chores