Skip to content

Commit da09d8d

Browse files
authored
Fix Scan Pull Request failing tests (#1240)
1 parent 84927cf commit da09d8d

File tree

3 files changed

+380
-120
lines changed

3 files changed

+380
-120
lines changed

scanpullrequest/scanpullrequest_test.go

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import (
3131
coreconfig "github.com/jfrog/jfrog-cli-core/v2/utils/config"
3232
"github.com/jfrog/jfrog-cli-security/utils/formats"
3333
"github.com/jfrog/jfrog-cli-security/utils/results"
34-
"github.com/jfrog/jfrog-client-go/utils/io/fileutils"
3534
"github.com/stretchr/testify/assert"
3635

3736
"github.com/jfrog/frogbot/v2/utils"
@@ -56,8 +55,11 @@ var basicConfigProfile = services2.ConfigProfile{
5655
ModuleName: "test-module",
5756
PathFromRoot: ".",
5857
ScanConfig: services2.ScanConfig{
59-
ScaScannerConfig: services2.ScaScannerConfig{EnableScaScan: true},
60-
SastScannerConfig: services2.SastScannerConfig{EnableSastScan: true},
58+
ScaScannerConfig: services2.ScaScannerConfig{EnableScaScan: true},
59+
SastScannerConfig: services2.SastScannerConfig{EnableSastScan: true},
60+
ContextualAnalysisScannerConfig: services2.CaScannerConfig{EnableCaScan: true},
61+
SecretsScannerConfig: services2.SecretsScannerConfig{EnableSecretsScan: true},
62+
IacScannerConfig: services2.IacScannerConfig{EnableIacScan: true},
6163
},
6264
},
6365
},
@@ -1353,19 +1355,11 @@ func preparePullRequestTest(t *testing.T, projectName string) (utils.Repository,
13531355
Target: vcsclient.BranchInfo{Name: testTargetBranchName, Repository: projectName, Owner: owner},
13541356
},
13551357
}
1356-
server := httptest.NewServer(createGitLabHandler(t, gitServerParams))
1357-
13581358
testDir, cleanUp := utils.CopyTestdataProjectsToTemp(t, "scanpullrequest")
1359+
server := httptest.NewServer(createGitLabHandler(t, testDir, gitServerParams))
13591360
config, client := prepareConfigAndClient(t, xrayVersion, xscVersion, server, params, gitServerParams)
13601361

1361-
// Renames test git folder to .git
1362-
currentDir := filepath.Join(testDir, projectName)
1363-
restoreDir, err := utils.Chdir(currentDir)
1364-
assert.NoError(t, err)
1365-
13661362
return config, client, func() {
1367-
assert.NoError(t, restoreDir())
1368-
assert.NoError(t, fileutils.RemoveTempDir(currentDir))
13691363
cleanUp()
13701364
server.Close()
13711365
restoreEnv()
@@ -1379,7 +1373,7 @@ type GitServerParams struct {
13791373
}
13801374

13811375
// Create HTTP handler to mock GitLab server
1382-
func createGitLabHandler(t *testing.T, params GitServerParams) http.HandlerFunc {
1376+
func createGitLabHandler(t *testing.T, testDir string, params GitServerParams) http.HandlerFunc {
13831377
return func(w http.ResponseWriter, r *http.Request) {
13841378
repoInfo := params.RepoOwner + "%2F" + params.RepoName
13851379
switch {
@@ -1389,28 +1383,26 @@ func createGitLabHandler(t *testing.T, params GitServerParams) http.HandlerFunc
13891383
// Mimic get pull request by ID
13901384
case r.RequestURI == fmt.Sprintf("/api/v4/projects/%s/merge_requests/%d", repoInfo, params.prDetails.ID):
13911385
w.WriteHeader(http.StatusOK)
1392-
// expectedResponse, err := os.ReadFile(filepath.Join("..", "expectedPullRequestDetailsResponse.json"))
1393-
// assert.NoError(t, err)
13941386
_, err := fmt.Fprintf(w, `{ "id": %d, "iid": 133, "project_id": 15513260, "title": "Dummy pull request", "description": "this is pr description", "state": "opened", "target_branch": "%s", "source_branch": "%s", "author": {"username": "testuser"}}`, params.prDetails.ID, params.prDetails.Target.Name, params.prDetails.Source.Name)
13951387
assert.NoError(t, err)
13961388
// Mimic download specific branch to scan
13971389
case r.RequestURI == fmt.Sprintf("/api/v4/projects/%s/repository/archive.tar.gz?sha=%s", repoInfo, params.prDetails.Source.Name):
13981390
w.WriteHeader(http.StatusOK)
1399-
repoFile, err := os.ReadFile(filepath.Join("..", params.RepoName, "sourceBranch.gz"))
1391+
repoFile, err := os.ReadFile(filepath.Join(testDir, params.RepoName, "sourceBranch.gz"))
14001392
assert.NoError(t, err)
14011393
_, err = w.Write(repoFile)
14021394
assert.NoError(t, err)
14031395
// Download repository mock
14041396
case r.RequestURI == fmt.Sprintf("/api/v4/projects/%s/repository/archive.tar.gz?sha=%s", repoInfo, params.prDetails.Target.Name):
14051397
w.WriteHeader(http.StatusOK)
1406-
repoFile, err := os.ReadFile(filepath.Join("..", params.RepoName, "targetBranch.gz"))
1398+
repoFile, err := os.ReadFile(filepath.Join(testDir, params.RepoName, "targetBranch.gz"))
14071399
assert.NoError(t, err)
14081400
_, err = w.Write(repoFile)
14091401
assert.NoError(t, err)
14101402
return
14111403
case r.RequestURI == fmt.Sprintf("/api/v4/projects/%s/merge_requests/133/notes", repoInfo) && r.Method == http.MethodGet:
14121404
w.WriteHeader(http.StatusOK)
1413-
comments, err := os.ReadFile(filepath.Join("..", "commits.json"))
1405+
comments, err := os.ReadFile(filepath.Join(testDir, "commits.json"))
14141406
assert.NoError(t, err)
14151407
_, err = w.Write(comments)
14161408
assert.NoError(t, err)
@@ -1430,11 +1422,10 @@ func createGitLabHandler(t *testing.T, params GitServerParams) http.HandlerFunc
14301422

14311423
var expectedResponse []byte
14321424
if strings.Contains(params.RepoName, "multi-dir") {
1433-
expectedResponse = outputwriter.GetJsonBodyOutputFromFile(t, filepath.Join("..", "expected_response_multi_dir.md"))
1425+
expectedResponse = outputwriter.GetJsonBodyOutputFromFile(t, filepath.Join(testDir, "expected_response_multi_dir.md"))
14341426
} else {
1435-
expectedResponse = outputwriter.GetJsonBodyOutputFromFile(t, filepath.Join("..", "expected_response.md"))
1427+
expectedResponse = outputwriter.GetJsonBodyOutputFromFile(t, filepath.Join(testDir, "expected_response.md"))
14361428
}
1437-
assert.NoError(t, err)
14381429
assert.JSONEq(t, string(expectedResponse), buf.String())
14391430

14401431
w.WriteHeader(http.StatusOK)
@@ -1445,7 +1436,7 @@ func createGitLabHandler(t *testing.T, params GitServerParams) http.HandlerFunc
14451436
_, err := w.Write([]byte(jsonResponse))
14461437
assert.NoError(t, err)
14471438
case r.RequestURI == fmt.Sprintf("/api/v4/projects/%s/merge_requests/133/discussions", repoInfo):
1448-
discussions, err := os.ReadFile(filepath.Join("..", "list_merge_request_discussion_items.json"))
1439+
discussions, err := os.ReadFile(filepath.Join(testDir, "list_merge_request_discussion_items.json"))
14491440
assert.NoError(t, err)
14501441
_, err = w.Write(discussions)
14511442
assert.NoError(t, err)

testdata/scanpullrequest/expected_response.md

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,9 @@
2323

2424
### 📦 Vulnerable Dependencies
2525

26-
<div align='center'>
27-
28-
| Severity | ID | Contextual Analysis | Direct Dependencies | Impacted Dependency | Fixed Versions |
29-
| :---------------------: | :-----------------------------------: | :-----------------------------------: | :-----------------------------------: | :-----------------------------------: | :-----------------------------------: |
30-
| ![critical (not applicable)](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/notApplicableCritical.png)<br>Critical | CVE-2021-44906 | Not Applicable | minimist:1.2.5 | minimist 1.2.5 | [0.2.4]<br>[1.2.6] |
31-
32-
</div>
33-
26+
| Severity | ID | Contextual Analysis | Dependency Path |
27+
| :---------------------: | :-----------------------------------: | :-----------------------------------: | ----------------------------------- |
28+
| ![critical (not applicable)](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/notApplicableCritical.png)<br>Critical | CVE-2021-44906 | Not Applicable | <details><summary><b>1 Direct</b></summary>minimist:1.2.5<br></details> |
3429

3530
### 🔖 Details
3631

@@ -39,36 +34,11 @@
3934
### Vulnerability Details
4035
| | |
4136
| --------------------- | :-----------------------------------: |
42-
| **Jfrog Research Severity:** | <img src="https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/smallHigh.svg" alt=""/> High |
4337
| **Contextual Analysis:** | Not Applicable |
44-
| **Direct Dependencies:** | minimist:1.2.5 |
45-
| **Impacted Dependency:** | minimist:1.2.5 |
46-
| **Fixed Versions:** | [0.2.4], [1.2.6] |
4738
| **CVSS V3:** | 9.8 |
39+
| **Dependency Path:** | <details><summary><b>minimist: 1.2.5 (Direct)</b></summary>Fix Version: 1.2.6<br></details> |
4840

49-
Insufficient input validation in Minimist npm package leads to prototype pollution of constructor functions when parsing arbitrary arguments.
50-
51-
### 🔬 JFrog Research Details
52-
53-
**Description:**
54-
[Minimist](https://github.com/substack/minimist) is a simple and very popular argument parser. It is used by more than 14 million by Mar 2022. This package developers stopped developing it since April 2020 and its community released a [newer version](https://github.com/meszaros-lajos-gyorgy/minimist-lite) supported by the community.
55-
56-
57-
An incomplete fix for [CVE-2020-7598](https://nvd.nist.gov/vuln/detail/CVE-2020-7598) partially blocked prototype pollution attacks. Researchers discovered that it does not check for constructor functions which means they can be overridden. This behavior can be triggered easily when using it insecurely (which is the common usage). For example:
58-
```
59-
var argv = parse(['--_.concat.constructor.prototype.y', '123']);
60-
t.equal((function(){}).foo, undefined);
61-
t.equal(argv.y, undefined);
62-
```
63-
In this example, `prototype.y` is assigned with `123` which will be derived to every newly created object.
64-
65-
This vulnerability can be triggered when the attacker-controlled input is parsed using Minimist without any validation. As always with prototype pollution, the impact depends on the code that follows the attack, but denial of service is almost always guaranteed.
66-
67-
**Remediation:**
68-
##### Development mitigations
69-
70-
Add the `Object.freeze(Object.prototype);` directive once at the beginning of your main JS source code file (ex. `index.js`), preferably after all your `require` directives. This will prevent any changes to the prototype object, thus completely negating prototype pollution attacks.
71-
41+
Minimist prior to 1.2.6 and 0.2.4 is vulnerable to Prototype Pollution via file `index.js`, function `setKey()` (lines 69-95).
7242

7343
---
7444
<div align='center'>

0 commit comments

Comments
 (0)