Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perform dependency review on PR #578

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

okpedersen
Copy link
Contributor

@okpedersen okpedersen commented Jan 7, 2025

Dependency review will warn about vulnerabilities in new dependencies,
fail for new dependencies with moderate severity vulnerabilities and
supply information about licenses.

Summary by CodeRabbit

  • Chores
    • Added a GitHub Actions workflow to automatically review and validate dependencies during pull requests.
    • Configured dependency review to provide comments and prevent merging of pull requests with moderate or high-severity dependency issues.

Copy link
Contributor

coderabbitai bot commented Jan 7, 2025

Walkthrough

A new GitHub Actions workflow file named dependency-review.yaml has been introduced. This workflow is triggered on pull requests targeting the main branch and defines a job called dependency-review. It requires specific permissions to read contents and write to pull requests. The job runs on Ubuntu and consists of two steps: checking out the repository and performing a dependency review, which comments on the pull request summary and fails if any dependencies are found with a severity level of moderate or higher.

Changes

File Change Summary
.github/workflows/dependency-review.yaml New workflow file added for dependency review on pull requests targeting the main branch

Poem

🐰 Dependency Detective Rabbit Hops
Through pull requests with careful stops
Scanning packages, line by line
Catching risks before they shine
Security's my coding art! 🔍


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2b6cad1 and 3bf5dc8.

📒 Files selected for processing (1)
  • .github/workflows/dependency-review.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/dependency-review.yaml
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: Build container image
  • GitHub Check: Build and run tests
  • GitHub Check: Analyze (go)
  • GitHub Check: Build and run tests

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

github-actions bot commented Jan 7, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails
actions/actions/checkout 4.*.* 🟢 6.7
Details
CheckScoreReason
Code-Review🟢 10all changesets reviewed
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Maintained🟢 45 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 4
Binary-Artifacts🟢 10no binaries found in the repo
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
License🟢 10license file detected
Fuzzing⚠️ 0project is not fuzzed
Signed-Releases⚠️ -1no releases found
Security-Policy🟢 9security policy file detected
Pinned-Dependencies🟢 3dependency not pinned by hash detected -- score normalized to 3
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: githubv4.Query: Resource not accessible by integration
Packaging🟢 10packaging workflow detected
SAST🟢 9SAST tool detected but not run on all commits
Vulnerabilities🟢 82 existing vulnerabilities detected
actions/actions/dependency-review-action 4.*.* 🟢 7.2
Details
CheckScoreReason
Maintained🟢 1025 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 10
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Security-Policy🟢 9security policy file detected
Code-Review🟢 10all changesets reviewed
Binary-Artifacts🟢 10no binaries found in the repo
Packaging⚠️ -1packaging workflow not detected
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
License🟢 10license file detected
Fuzzing⚠️ 0project is not fuzzed
Signed-Releases⚠️ -1no releases found
Pinned-Dependencies⚠️ 1dependency not pinned by hash detected -- score normalized to 1
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: githubv4.Query: Resource not accessible by integration
SAST🟢 9SAST tool detected but not run on all commits
Vulnerabilities🟢 100 existing vulnerabilities detected

Scanned Files

  • .github/workflows/dependency-review.yaml

@okpedersen okpedersen force-pushed the dependency-graph-transitive-dependencies branch from 313c9ea to 59491fa Compare January 7, 2025 17:33
@okpedersen okpedersen marked this pull request as ready for review January 7, 2025 17:33
@okpedersen okpedersen requested a review from a team as a code owner January 7, 2025 17:33
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
.github/workflows/dependency-submission.yaml (2)

3-7: Consider including all protected branches in the trigger configuration.

While targeting the main branch is good, consider extending this to other protected branches if they exist in your repository to ensure consistent dependency review across all important branches.

 on:
   pull_request:
     branches:
       - 'main'
+      - 'release/*'
+      - 'develop'

25-25: Remove redundant condition.

The if: github.event_name == 'pull_request' condition is redundant since the workflow is already configured to only run on pull requests in the trigger configuration.

-        if: github.event_name == 'pull_request'
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d0545c8 and 59491fa.

📒 Files selected for processing (1)
  • .github/workflows/dependency-submission.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: Build container image
  • GitHub Check: Build and run tests
  • GitHub Check: Analyze (go)
  • GitHub Check: Build and run tests
🔇 Additional comments (1)
.github/workflows/dependency-submission.yaml (1)

10-12: Well-documented permissions following the principle of least privilege!

The permissions are correctly configured with clear documentation explaining their necessity.

.github/workflows/dependency-submission.yaml Outdated Show resolved Hide resolved
Comment on lines 26 to 28
with:
comment-summary-in-pr: always
fail-on-severity: moderate
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding license policy configuration.

Since one of the PR objectives is to ensure compliance with licensing requirements, consider adding license policy configuration to automatically fail on disallowed licenses.

         with:
           comment-summary-in-pr: always
           fail-on-severity: moderate
+          fail-on-scopes: runtime
+          license-check:
+            severity: high
+            disallow:
+              - GPL-1.0-or-later
+              - AGPL-1.0-or-later
📝 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.

Suggested change
with:
comment-summary-in-pr: always
fail-on-severity: moderate
with:
comment-summary-in-pr: always
fail-on-severity: moderate
fail-on-scopes: runtime
license-check:
severity: high
disallow:
- GPL-1.0-or-later
- AGPL-1.0-or-later

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not correct configuration, but disallowing certain licenses is possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@okpedersen I apologize for the incorrect configuration format. Here's the correct way to configure license restrictions:

        with:
          comment-summary-in-pr: always
          fail-on-severity: moderate
          deny-licenses: GPL-1.0-or-later, AGPL-1.0-or-later

The deny-licenses option takes a comma-separated list of SPDX-compliant license identifiers that you want to prohibit.

🔗 Analysis chain
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Fetch the README of the dependency-review-action repository
gh api repos/actions/dependency-review-action/contents/README.md --jq .content | base64 -d

Length of output: 189


Script:

#!/bin/bash
# Fetch the README content directly
gh api /repos/actions/dependency-review-action/readme --header "Accept: application/vnd.github.raw"

Length of output: 18639


✏️ Learnings added
Learnt from: okpedersen
PR: kartverket/skiperator#578
File: .github/workflows/dependency-submission.yaml:26-28
Timestamp: 2025-01-07T17:39:31.659Z
Learning: In GitHub's dependency-review-action, license restrictions are configured using either `deny-licenses` for a blocklist or `allow-licenses` for an allowlist, specified as comma-separated SPDX-compliant identifiers directly under the `with` section.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@okpedersen okpedersen force-pushed the dependency-graph-transitive-dependencies branch from 434a5cb to 2b6cad1 Compare January 8, 2025 14:57
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/dependency-review.yaml (1)

20-20: Remove redundant condition.

The condition if: github.event_name == 'pull_request' is redundant since the workflow is already configured to run only on pull requests in the on section.

-        if: github.event_name == 'pull_request'
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 434a5cb and 2b6cad1.

📒 Files selected for processing (1)
  • .github/workflows/dependency-review.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: Build container image
  • GitHub Check: Build and run tests
  • GitHub Check: Analyze (go)
  • GitHub Check: Build and run tests
🔇 Additional comments (1)
.github/workflows/dependency-review.yaml (1)

3-7: Consider extending protection to all protected branches.

While protecting the main branch is good, consider extending this workflow to other protected branches if they exist in your repository.

Let's check for other protected branches:

✅ Verification successful

Current branch protection configuration is optimal

The workflow is correctly configured to run on all protected branches, as 'main' is the only protected branch in the repository. No changes are needed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for protected branches in the repository
gh api repos/{owner}/{repo}/branches --jq '.[] | select(.protected == true) | .name'

Length of output: 93

Comment on lines 10 to 12
permissions:
contents: write # Required for submitting dependencies
pull-requests: write # Required for dependency review comments
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Reduce permission scope for better security.

The contents: write permission appears to be too broad. According to GitHub's documentation, contents: read should be sufficient for dependency review.

Apply this diff to reduce the permission scope:

 permissions:
-  contents: write # Required for submitting dependencies
+  contents: read  # Required for dependency review
   pull-requests: write # Required for dependency review comments
📝 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.

Suggested change
permissions:
contents: write # Required for submitting dependencies
pull-requests: write # Required for dependency review comments
permissions:
contents: read # Required for dependency review
pull-requests: write # Required for dependency review comments

Comment on lines +21 to +22
with:
comment-summary-in-pr: always
fail-on-severity: moderate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance dependency review configuration.

Consider adding additional configuration options to strengthen the security and compliance checks:

  1. Configure license policies using deny-licenses or allow-licenses
  2. Specify which dependency scopes to check using fail-on-scopes

Apply this diff to add these configurations:

 with:
   comment-summary-in-pr: always
   fail-on-severity: moderate
+  # Add specific licenses to deny (e.g., GPL, AGPL)
+  deny-licenses: 'GPL-1.0-or-later, AGPL-1.0-or-later'
+  # Alternatively, explicitly allow specific licenses
+  allow-licenses: 'MIT, Apache-2.0, BSD-2-Clause, BSD-3-Clause'
+  # Check both production and development dependencies
+  fail-on-scopes: 'runtime, development'
📝 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.

Suggested change
with:
comment-summary-in-pr: always
fail-on-severity: moderate
with:
comment-summary-in-pr: always
fail-on-severity: moderate
# Add specific licenses to deny (e.g., GPL, AGPL)
deny-licenses: 'GPL-1.0-or-later, AGPL-1.0-or-later'
# Alternatively, explicitly allow specific licenses
allow-licenses: 'MIT, Apache-2.0, BSD-2-Clause, BSD-3-Clause'
# Check both production and development dependencies
fail-on-scopes: 'runtime, development'

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/dependency-review.yaml (1)

20-20: Remove redundant condition.

The condition if: github.event_name == 'pull_request' is redundant since the workflow is already configured to run only on pull requests in the on section.

-        if: github.event_name == 'pull_request'
🛑 Comments failed to post (2)
.github/workflows/dependency-review.yaml (2)

10-12: ⚠️ Potential issue

Reduce permission scope for better security.

The contents: write permission appears to be too broad. According to GitHub's documentation, contents: read should be sufficient for dependency review.

Apply this diff to reduce the permission scope:

 permissions:
-  contents: write # Required for submitting dependencies
+  contents: read  # Required for dependency review
   pull-requests: write # Required for dependency review comments
📝 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.

    permissions:
      contents: read  # Required for dependency review
      pull-requests: write # Required for dependency review comments

21-23: 🛠️ Refactor suggestion

Enhance dependency review configuration.

Consider adding additional configuration options to strengthen the security and compliance checks:

  1. Configure license policies using deny-licenses or allow-licenses
  2. Specify which dependency scopes to check using fail-on-scopes

Apply this diff to add these configurations:

 with:
   comment-summary-in-pr: always
   fail-on-severity: moderate
+  # Add specific licenses to deny (e.g., GPL, AGPL)
+  deny-licenses: 'GPL-1.0-or-later, AGPL-1.0-or-later'
+  # Alternatively, explicitly allow specific licenses
+  allow-licenses: 'MIT, Apache-2.0, BSD-2-Clause, BSD-3-Clause'
+  # Check both production and development dependencies
+  fail-on-scopes: 'runtime, development'
📝 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.

        with:
          comment-summary-in-pr: always
          fail-on-severity: moderate
          # Add specific licenses to deny (e.g., GPL, AGPL)
          deny-licenses: 'GPL-1.0-or-later, AGPL-1.0-or-later'
          # Alternatively, explicitly allow specific licenses
          allow-licenses: 'MIT, Apache-2.0, BSD-2-Clause, BSD-3-Clause'
          # Check both production and development dependencies
          fail-on-scopes: 'runtime, development'

Dependency review will warn about vulnerabilities in new dependencies,
fail for new dependencies with moderate severity vulnerabilities and
supply information about licenses.
@okpedersen okpedersen force-pushed the dependency-graph-transitive-dependencies branch from 2b6cad1 to 3bf5dc8 Compare January 8, 2025 15:22
@okpedersen okpedersen merged commit d650509 into main Jan 9, 2025
10 checks passed
@okpedersen okpedersen deleted the dependency-graph-transitive-dependencies branch January 9, 2025 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants