Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 40 additions & 20 deletions integrationutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ package main
import (
"context"
"fmt"
"os"
"strconv"
"strings"
"testing"
"time"

"github.com/go-git/go-git/v5"
githttp "github.com/go-git/go-git/v5/plumbing/transport/http"
"github.com/jfrog/frogbot/v2/scanpullrequest"
Expand All @@ -13,11 +19,6 @@ import (
"github.com/jfrog/froggit-go/vcsutils"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"os"
"strconv"
"strings"
"testing"
"time"
)

const (
Expand Down Expand Up @@ -76,18 +77,19 @@ func setIntegrationTestEnvs(t *testing.T, testDetails *IntegrationTestDetails) f
// so we restore them at the end of the test to avoid collisions with other tests
envRestoreFunc := getJfrogEnvRestoreFunc(t)
unsetEnvs := utils.SetEnvsAndAssertWithCallback(t, map[string]string{
utils.RequirementsFileEnv: "requirements.txt",
utils.GitPullRequestIDEnv: testDetails.PullRequestID,
utils.GitProvider: testDetails.GitProvider,
utils.GitTokenEnv: testDetails.GitToken,
utils.GitRepoEnv: testDetails.RepoName,
utils.GitRepoOwnerEnv: testDetails.RepoOwner,
utils.BranchNameTemplateEnv: testDetails.CustomBranchName,
utils.GitApiEndpointEnv: testDetails.ApiEndpoint,
utils.GitProjectEnv: testDetails.GitProject,
utils.GitUsernameEnv: testDetails.GitUsername,
utils.GitBaseBranchEnv: mainBranch,
utils.GitUseLocalRepositoryEnv: fmt.Sprintf("%t", testDetails.UseLocalRepo),
utils.RequirementsFileEnv: "requirements.txt",
utils.GitPullRequestIDEnv: testDetails.PullRequestID,
utils.GitProvider: testDetails.GitProvider,
utils.GitTokenEnv: testDetails.GitToken,
utils.GitRepoEnv: testDetails.RepoName,
utils.GitRepoOwnerEnv: testDetails.RepoOwner,
utils.BranchNameTemplateEnv: testDetails.CustomBranchName,
utils.GitApiEndpointEnv: testDetails.ApiEndpoint,
utils.GitProjectEnv: testDetails.GitProject,
utils.GitUsernameEnv: testDetails.GitUsername,
utils.GitBaseBranchEnv: mainBranch,
utils.GitUseLocalRepositoryEnv: fmt.Sprintf("%t", testDetails.UseLocalRepo),
utils.IncludeVulnerabilitiesEnv: "TRUE",
})
return func() {
envRestoreFunc()
Expand Down Expand Up @@ -232,6 +234,15 @@ func validateResults(t *testing.T, ctx context.Context, client vcsclient.VcsClie
comments, err := client.ListPullRequestComments(ctx, testDetails.RepoOwner, testDetails.RepoName, prID)
require.NoError(t, err)

t.Logf("ListPullRequestComments returned %d comments", len(comments))
for i, comment := range comments {
contentPreview := comment.Content
if len(contentPreview) > 100 {
contentPreview = contentPreview[:100] + "..."
}
t.Logf("Comment %d: ID=%d, Content preview: %s", i, comment.ID, contentPreview)
}

switch actualClient := client.(type) {
case *vcsclient.GitHubClient:
validateGitHubComments(t, ctx, actualClient, testDetails, prID, comments)
Expand All @@ -240,7 +251,7 @@ func validateResults(t *testing.T, ctx context.Context, client vcsclient.VcsClie
case *vcsclient.BitbucketServerClient:
validateBitbucketServerComments(t, comments)
case *vcsclient.GitLabClient:
validateGitLabComments(t, comments)
validateGitLabComments(t, ctx, actualClient, testDetails, prID, comments)
}
}

Expand All @@ -264,9 +275,18 @@ func validateBitbucketServerComments(t *testing.T, comments []vcsclient.CommentI
assertBannerExists(t, comments, outputwriter.GetSimplifiedTitle(outputwriter.VulnerabilitiesPrBannerSource))
}

func validateGitLabComments(t *testing.T, comments []vcsclient.CommentInfo) {
assert.GreaterOrEqual(t, len(comments), expectedNumberOfIssues)
func validateGitLabComments(t *testing.T, ctx context.Context, client *vcsclient.GitLabClient, testDetails *IntegrationTestDetails, prID int, comments []vcsclient.CommentInfo) {
// GitLab separates regular comments (notes) from review comments (discussions)
// The summary comment with banner should be in regular comments
t.Logf("Regular comments (notes): %d", len(comments))
assertBannerExists(t, comments, string(outputwriter.VulnerabilitiesMrBannerSource))

// Review comments are stored as discussions
reviewComments, err := client.ListPullRequestReviewComments(ctx, testDetails.RepoOwner, testDetails.RepoName, prID)
assert.NoError(t, err)
t.Logf("Review comments (discussions): %d", len(reviewComments))
// We expect at least the review comments (IAC, SAST, Applicable)
assert.GreaterOrEqual(t, len(reviewComments), 1)
}

func getJfrogEnvRestoreFunc(t *testing.T) func() {
Expand Down
12 changes: 6 additions & 6 deletions scanpullrequest/scanpullrequest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func TestScanResultsToIssuesCollection(t *testing.T) {
Applicable: "Applicable",
FixedVersions: []string{"1.2.3"},
ImpactedDependencyDetails: formats.ImpactedDependencyDetails{
SeverityDetails: formats.SeverityDetails{Severity: "High", SeverityNumValue: 21},
SeverityDetails: formats.SeverityDetails{Severity: "High", SeverityNumValue: 26},
ImpactedDependencyName: "Dep-1",
},
Cves: []formats.CveRow{{Id: "CVE-2022-2122", Applicability: &formats.Applicability{Status: "Applicable", ScannerDescription: "rule-msg", Evidence: []formats.Evidence{{Reason: "result-msg", Location: formats.Location{File: "file1", StartLine: 1, StartColumn: 10, EndLine: 2, EndColumn: 11, Snippet: "snippet"}}}}}},
Expand All @@ -107,7 +107,7 @@ func TestScanResultsToIssuesCollection(t *testing.T) {
Applicable: "Not Applicable",
FixedVersions: []string{"1.2.2"},
ImpactedDependencyDetails: formats.ImpactedDependencyDetails{
SeverityDetails: formats.SeverityDetails{Severity: "Low", SeverityNumValue: 2},
SeverityDetails: formats.SeverityDetails{Severity: "Low", SeverityNumValue: 3},
ImpactedDependencyName: "Dep-2",
},
Cves: []formats.CveRow{{Id: "CVE-2023-3122", Applicability: &formats.Applicability{Status: "Not Applicable", ScannerDescription: "rule-msg"}}},
Expand All @@ -117,7 +117,7 @@ func TestScanResultsToIssuesCollection(t *testing.T) {
{
SeverityDetails: formats.SeverityDetails{
Severity: "High",
SeverityNumValue: 21,
SeverityNumValue: 26,
},
ScannerInfo: formats.ScannerInfo{
ScannerDescription: "rule-msg",
Expand All @@ -138,7 +138,7 @@ func TestScanResultsToIssuesCollection(t *testing.T) {
{
SeverityDetails: formats.SeverityDetails{
Severity: "High",
SeverityNumValue: 21,
SeverityNumValue: 26,
},
ScannerInfo: formats.ScannerInfo{
ScannerDescription: "rule-msg",
Expand All @@ -159,7 +159,7 @@ func TestScanResultsToIssuesCollection(t *testing.T) {
{
SeverityDetails: formats.SeverityDetails{
Severity: "High",
SeverityNumValue: 21,
SeverityNumValue: 26,
},
ScannerInfo: formats.ScannerInfo{
ScannerDescription: "rule-msg",
Expand All @@ -183,7 +183,7 @@ func TestScanResultsToIssuesCollection(t *testing.T) {
ImpactedDependencyDetails: formats.ImpactedDependencyDetails{
SeverityDetails: formats.SeverityDetails{
Severity: "Medium",
SeverityNumValue: 14,
SeverityNumValue: 19,
},
ImpactedDependencyName: "Dep-1",
},
Expand Down
31 changes: 18 additions & 13 deletions testdata/messages/integration/test_proj_pip_with_vulnerability.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@


## 📗 Scan Summary
- Frogbot scanned for vulnerabilities and found 1 issues
- Frogbot scanned for vulnerabilities and found 2 issues

| Scan Category | Status | Security Issues |
| --------------------- | :-----------------------------------: | ----------------------------------- |
| **Software Composition Analysis** | ✅ Done | <details><summary><b>1 Issues Found</b></summary><img src="https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/smallHigh.svg" alt=""/> 1 High<br></details> |
| **Software Composition Analysis** | ✅ Done | <details><summary><b>2 Issues Found</b></summary><img src="https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/smallHigh.svg" alt=""/> 2 High<br></details> |
| **Contextual Analysis** | ✅ Done | - |
| **Static Application Security Testing (SAST)** | ✅ Done | Not Found |
| **Secrets** | ✅ Done | - |
Expand All @@ -28,13 +28,15 @@
| Severity | ID | Contextual Analysis | Direct Dependencies | Impacted Dependency | Fixed Versions |
| :---------------------: | :-----------------------------------: | :-----------------------------------: | :-----------------------------------: | :-----------------------------------: | :-----------------------------------: |
| ![high](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)<br> High | CVE-2022-29217 | Not Covered | pyjwt:1.7.1 | pyjwt 1.7.1 | [2.4.0] |
| ![high (not applicable)](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/notApplicableHigh.png)<br> High | CVE-2025-45768 | Not Applicable | pyjwt:1.7.1 | pyjwt 1.7.1 | - |

</div>


### 🔖 Details


<details><summary><b>[ CVE-2022-29217 ] pyjwt 1.7.1</b></summary>

### Vulnerability Details
| | |
Expand Down Expand Up @@ -67,17 +69,7 @@ For example, an application might have planned to validate an `EdDSA`-signed tok
# Making a good jwt token that should work by signing it with the private key
encoded_good = jwt.encode({"test": 1234}, priv_key_bytes, algorithm="EdDSA")
```
An attacker in posession of the public key can generate an `HMAC`-signed token to confuse PyJWT -
```python
# Using HMAC with the public key to trick the receiver to think that the public key is a HMAC secret
encoded_bad = jwt.encode({"test": 1234}, pub_key_bytes, algorithm="HS256")
```

The following vulnerable `decode` call will accept BOTH of the above tokens as valid -
```
decoded = jwt.decode(encoded_good, pub_key_bytes,
algorithms=jwt.algorithms.get_default_algorithms())
```
A...

**Remediation:**
##### Development mitigations
Expand All @@ -87,7 +79,20 @@ For example, replace the following call -
`jwt.decode(encoded_jwt, pub_key_bytes, algorithms=jwt.algorithms.get_default_algorithms())`
With -
`jwt.decode(encoded_jwt, pub_key_bytes, algorithms=["ES256"])`
<br></details>

<details><summary><b>[ CVE-2025-45768 ] pyjwt 1.7.1</b></summary>

### Vulnerability Details
| | |
| --------------------- | :-----------------------------------: |
| **Contextual Analysis:** | Not Applicable |
| **Direct Dependencies:** | pyjwt:1.7.1 |
| **Impacted Dependency:** | pyjwt:1.7.1 |
| **Fixed Versions:** | - |
| **CVSS V3:** | 7.0 |

pyjwt v2.10.1 was discovered to contain weak encryption. NOTE: this is disputed by the Supplier because the key length is chosen by the application that uses the library (admittedly, library users may benefit from a minimum value and a mechanism for opting in to strict enforcement).<br></details>


---
Expand Down
30 changes: 17 additions & 13 deletions testdata/scanpullrequest/expected_response_multi_dir.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@


## 📗 Scan Summary
- Frogbot scanned for vulnerabilities and found 2 issues
- Frogbot scanned for vulnerabilities and found 3 issues

| Scan Category | Status | Security Issues |
| --------------------- | :-----------------------------------: | ----------------------------------- |
| **Software Composition Analysis** | ✅ Done | <details><summary><b>2 Issues Found</b></summary><img src="https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/smallHigh.svg" alt=""/> 2 High<br></details> |
| **Software Composition Analysis** | ✅ Done | <details><summary><b>3 Issues Found</b></summary><img src="https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/smallHigh.svg" alt=""/> 3 High<br></details> |
| **Contextual Analysis** | ✅ Done | - |
| **Static Application Security Testing (SAST)** | ✅ Done | Not Found |
| **Secrets** | ✅ Done | - |
Expand All @@ -29,6 +29,7 @@
| :---------------------: | :-----------------------------------: | :-----------------------------------: | :-----------------------------------: | :-----------------------------------: | :-----------------------------------: |
| ![high (not applicable)](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/notApplicableHigh.png)<br> High | CVE-2022-3517 | Not Applicable | minimatch:3.0.4 | minimatch 3.0.4 | [3.0.5] |
| ![high](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)<br> High | CVE-2022-29217 | Not Covered | pyjwt:1.7.1 | pyjwt 1.7.1 | [2.4.0] |
| ![high (not applicable)](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/notApplicableHigh.png)<br> High | CVE-2025-45768 | Not Applicable | pyjwt:1.7.1 | pyjwt 1.7.1 | - |

</div>

Expand Down Expand Up @@ -82,17 +83,7 @@ For example, an application might have planned to validate an `EdDSA`-signed tok
# Making a good jwt token that should work by signing it with the private key
encoded_good = jwt.encode({"test": 1234}, priv_key_bytes, algorithm="EdDSA")
```
An attacker in posession of the public key can generate an `HMAC`-signed token to confuse PyJWT -
```python
# Using HMAC with the public key to trick the receiver to think that the public key is a HMAC secret
encoded_bad = jwt.encode({"test": 1234}, pub_key_bytes, algorithm="HS256")
```

The following vulnerable `decode` call will accept BOTH of the above tokens as valid -
```
decoded = jwt.decode(encoded_good, pub_key_bytes,
algorithms=jwt.algorithms.get_default_algorithms())
```
A...

**Remediation:**
##### Development mitigations
Expand All @@ -104,6 +95,19 @@ With -
`jwt.decode(encoded_jwt, pub_key_bytes, algorithms=["ES256"])`
<br></details>

<details><summary><b>[ CVE-2025-45768 ] pyjwt 1.7.1</b></summary>

### Vulnerability Details
| | |
| --------------------- | :-----------------------------------: |
| **Contextual Analysis:** | Not Applicable |
| **Direct Dependencies:** | pyjwt:1.7.1 |
| **Impacted Dependency:** | pyjwt:1.7.1 |
| **Fixed Versions:** | - |
| **CVSS V3:** | 7.0 |

pyjwt v2.10.1 was discovered to contain weak encryption. NOTE: this is disputed by the Supplier because the key length is chosen by the application that uses the library (admittedly, library users may benefit from a minimum value and a mechanism for opting in to strict enforcement).<br></details>


---
<div align='center'>
Expand Down
4 changes: 2 additions & 2 deletions utils/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ func HandlePullRequestCommentsAfterScan(issues *issues.ScansIssuesCollection, re
}

// Add summary (SCA, license) scan comment
if issues.IssuesExists(repo.PullRequestSecretComments) || repo.AddPrCommentOnSuccess {
for _, comment := range generatePullRequestSummaryComment(*issues, resultContext, repo.PullRequestSecretComments, repo.OutputWriter) {
if issues.IssuesExists(repo.Params.PullRequestSecretComments) || repo.AddPrCommentOnSuccess {
for _, comment := range generatePullRequestSummaryComment(*issues, resultContext, repo.Params.PullRequestSecretComments, repo.OutputWriter) {
if err = client.AddPullRequestComment(context.Background(), repo.RepoOwner, repo.RepoName, comment, pullRequestID); err != nil {
err = errors.New("couldn't add pull request comment: " + err.Error())
return
Expand Down