Skip to content

Commit 200e4c2

Browse files
committed
fixes bug, that a scanner will get removed all the time since we are appending another scanner with a colon
1 parent e8de889 commit 200e4c2

5 files changed

Lines changed: 113 additions & 3 deletions

File tree

internal/core/assetversion/asset_version_service.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ func diffScanResults(currentScanner string, foundVulnerabilities []models.Depend
368368

369369
// Now we work on the vulnerabilities found in both sets -> has the vulnerability this scanner id already in his scanner_ids
370370
for i := range foundByScannerAndExisting {
371-
if !strings.Contains(foundByScannerAndExisting[i].ScannerIDs, currentScanner) {
371+
if !utils.ContainsInWhitespaceSeparatedStringList(foundByScannerAndExisting[i].ScannerIDs, currentScanner) {
372372
detectedByOtherScanner = append(detectedByOtherScanner, foundByScannerAndExisting[i])
373373
}
374374
}
@@ -377,7 +377,7 @@ func diffScanResults(currentScanner string, foundVulnerabilities []models.Depend
377377
for i := range notFoundByScannerAndExisting {
378378
if strings.TrimSpace(notFoundByScannerAndExisting[i].ScannerIDs) == currentScanner {
379379
fixedVulns = append(fixedVulns, notFoundByScannerAndExisting[i])
380-
} else if strings.Contains(notFoundByScannerAndExisting[i].ScannerIDs, currentScanner) {
380+
} else if utils.ContainsInWhitespaceSeparatedStringList(notFoundByScannerAndExisting[i].ScannerIDs, currentScanner) {
381381
notDetectedByScannerAnymore = append(notDetectedByScannerAnymore, notFoundByScannerAndExisting[i])
382382
}
383383
}

internal/core/assetversion/asset_version_service_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,25 @@ func TestDiffScanResults(t *testing.T) {
245245
assert.Empty(t, detectedByCurrentScanner)
246246
assert.Empty(t, notDetectedByCurrentScannerAnymore)
247247
})
248+
249+
t.Run("BUG: should NOT incorrectly identify scanner removal when scanner ID contains colon and is substring of existing scanner", func(t *testing.T) {
250+
251+
currentScanner := "container-scanning"
252+
foundVulnerabilities := []models.DependencyVuln{
253+
{CVEID: utils.Ptr("CVE-1234")},
254+
}
255+
256+
existingDependencyVulns := []models.DependencyVuln{
257+
{CVEID: utils.Ptr("CVE-1234"), Vulnerability: models.Vulnerability{ScannerIDs: "github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner"}},
258+
}
259+
260+
foundByScannerAndNotExisting, fixedVulns, detectedByCurrentScanner, notDetectedByCurrentScannerAnymore := diffScanResults(currentScanner, foundVulnerabilities, existingDependencyVulns)
261+
262+
assert.Empty(t, foundByScannerAndNotExisting, "Should be empty - this is a new detection by current scanner")
263+
assert.Empty(t, fixedVulns, "Should be empty - no vulnerabilities are fixed")
264+
assert.Equal(t, 1, len(detectedByCurrentScanner), "Should detect that current scanner found existing vulnerability for first time")
265+
assert.Empty(t, notDetectedByCurrentScannerAnymore, "BUG: Should be empty - current scanner was never detecting this vulnerability before!")
266+
})
248267
}
249268

250269
func TestYamlMetadata(t *testing.T) {

internal/database/repositories/component_repository.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ func (c *componentRepository) HandleStateDiff(tx core.DB, assetVersionName strin
336336

337337
//Next step is adding the scanner id to all existing component dependencies we just found
338338
for i := range needToBeChanged {
339-
if !strings.Contains(needToBeChanged[i].ScannerIDs, scannerID) {
339+
if !utils.ContainsInWhitespaceSeparatedStringList(needToBeChanged[i].ScannerIDs, scannerID) {
340340
needToBeChanged[i].ScannerIDs = utils.AddToWhitespaceSeparatedStringList(needToBeChanged[i].ScannerIDs, scannerID)
341341
}
342342
}

internal/utils/common.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,11 @@ func RemoveFromWhitespaceSeparatedStringList(s string, item string) string {
172172
return strings.Join(res, " ")
173173
}
174174

175+
func ContainsInWhitespaceSeparatedStringList(s string, item string) bool {
176+
els := strings.Fields(s)
177+
return slices.Contains(els, item)
178+
}
179+
175180
func CompareFirstTwoDecimals(a, b float64) bool {
176181

177182
aRounded := math.Round(a*100) / 100

internal/utils/common_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,18 @@ func TestAddToWhitespaceSeparatedStringList(t *testing.T) {
3232
item: "item1",
3333
expected: "item1 item2",
3434
},
35+
{
36+
name: "Add scanner ID with colon",
37+
input: "scanner1",
38+
item: "github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
39+
expected: "scanner1 github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
40+
},
41+
{
42+
name: "Should not add duplicate scanner ID with colon",
43+
input: "github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
44+
item: "github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
45+
expected: "github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
46+
},
3547
}
3648

3749
for _, tt := range tests {
@@ -75,6 +87,18 @@ func TestRemoveFromWhitespaceSeparatedStringList(t *testing.T) {
7587
item: "item1",
7688
expected: "",
7789
},
90+
{
91+
name: "Remove scanner ID with colon from list",
92+
input: "github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
93+
item: "github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
94+
expected: "",
95+
},
96+
{
97+
name: "Should not remove partial match with colon",
98+
input: "container-scanning github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
99+
item: "container-scanning:scanner",
100+
expected: "container-scanning github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
101+
},
78102
}
79103

80104
for _, tt := range tests {
@@ -86,6 +110,68 @@ func TestRemoveFromWhitespaceSeparatedStringList(t *testing.T) {
86110
})
87111
}
88112
}
113+
114+
func TestContainsInWhitespaceSeparatedStringList(t *testing.T) {
115+
tests := []struct {
116+
name string
117+
input string
118+
item string
119+
expected bool
120+
}{
121+
{
122+
name: "Item exists in list",
123+
input: "item1 item2 item3",
124+
item: "item2",
125+
expected: true,
126+
},
127+
{
128+
name: "Item does not exist in list",
129+
input: "item1 item2 item3",
130+
item: "item4",
131+
expected: false,
132+
},
133+
{
134+
name: "Item exists in single-item list",
135+
input: "item1",
136+
item: "item1",
137+
expected: true,
138+
},
139+
{
140+
name: "Item does not exist in empty list",
141+
input: "",
142+
item: "item1",
143+
expected: false,
144+
},
145+
{
146+
name: "Scanner ID with colon exists in list",
147+
input: "github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
148+
item: "github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
149+
expected: true,
150+
},
151+
{
152+
name: "Should not match partial scanner ID with colon",
153+
input: "github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
154+
item: "container-scanning",
155+
expected: false,
156+
},
157+
{
158+
name: "Should not match partial scanner ID with colon (reverse)",
159+
input: "container-scanning github.com/l3montree-dev/devguard/cmd/devguard-scanner/container-scanning:scanner",
160+
item: "container-scanning:scanner",
161+
expected: false,
162+
},
163+
}
164+
165+
for _, tt := range tests {
166+
t.Run(tt.name, func(t *testing.T) {
167+
result := ContainsInWhitespaceSeparatedStringList(tt.input, tt.item)
168+
if result != tt.expected {
169+
t.Errorf("expected %t, got %t", tt.expected, result)
170+
}
171+
})
172+
}
173+
}
174+
89175
func TestShannonEntropy(t *testing.T) {
90176
tests := []struct {
91177
name string

0 commit comments

Comments
 (0)