Skip to content

Commit 084c1ae

Browse files
vamshi-stepsecurityvamshi-stepsecurity
authored
Merge pull request step-security#2605 from vamshi-stepsecurity/bug/pdpr/support-dpb-directories
handle mulitple directories to add scenario
2 parents 5ea07d9 + ec74a28 commit 084c1ae

6 files changed

Lines changed: 270 additions & 33 deletions

File tree

‎remediation/dependabot/dependabotconfig.go‎

Lines changed: 65 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"errors"
77
"fmt"
8+
"sort"
89
"strings"
910

1011
dependabot "github.com/paulvollmer/dependabot-config-go"
@@ -314,6 +315,11 @@ func UpdateDependabotConfig(dependabotConfig string) (*UpdateDependabotConfigRes
314315
updatesLastLine := findLastLine(updatesNode)
315316
lineOffset := 0
316317

318+
// First pass: collect directories to append per existing config entry.
319+
// This avoids calling replaceSequence multiple times on the same YAML
320+
// node, which would corrupt line positions.
321+
dirsToAppend := map[int][]string{} // cfg.Updates index -> dirs to add
322+
var newEntries []Ecosystem
317323
for _, eco := range updateDependabotConfigRequest.Ecosystems {
318324
updateAlreadyExist := false
319325
for _, update := range cfg.Updates {
@@ -324,50 +330,63 @@ func UpdateDependabotConfig(dependabotConfig string) (*UpdateDependabotConfigRes
324330
}
325331

326332
if !updateAlreadyExist {
327-
// If an existing entry uses directories (plural) for the same ecosystem,
328-
// append the new directory to that list instead of creating a new entry.
329-
appendedToDirectories := false
333+
appended := false
330334
for i, update := range cfg.Updates {
331335
if update.PackageEcosystem == eco.PackageEcosystem && len(update.Directories) > 0 {
332-
entryNode := updatesNode.Content[i]
333-
dirsNode := findMappingValue(entryNode, "directories")
334-
if dirsNode != nil {
335-
newDirs := append(update.Directories, eco.Directory)
336-
newLines, netChange, ch := replaceSequence(inputLines, dirsNode, newDirs, lineOffset)
337-
if ch {
338-
inputLines = newLines
339-
lineOffset += netChange
340-
response.IsChanged = true
341-
}
342-
appendedToDirectories = true
343-
}
336+
dirsToAppend[i] = append(dirsToAppend[i], eco.Directory)
337+
appended = true
344338
break
345339
}
346340
}
341+
if !appended {
342+
newEntries = append(newEntries, eco)
343+
}
344+
}
345+
}
347346

348-
if !appendedToDirectories {
349-
item := ecosystemToExtendedUpdate(eco)
350-
items := []ExtendedUpdate{item}
351-
addedItem, err := yaml.Marshal(items)
352-
if err != nil {
353-
return nil, fmt.Errorf("failed to marshal update items: %v", err)
354-
}
355-
data, err := addIndentation(string(addedItem), indentation)
356-
if err != nil {
357-
return nil, fmt.Errorf("failed to add indentation: %v", err)
358-
}
359-
360-
// Trim trailing newline to avoid double blank lines when content
361-
// follows after the updates section (e.g. registries block).
362-
dataLines := strings.Split(strings.TrimRight(data, "\n"), "\n")
363-
insertAt := updatesLastLine + lineOffset
364-
inputLines = insertAfterLine(inputLines, insertAt, dataLines)
365-
lineOffset += len(dataLines)
347+
// Apply collected directory appends — one replaceSequence call per entry.
348+
// Sort indices so we process top-to-bottom; lineOffset stays correct.
349+
sortedIndices := make([]int, 0, len(dirsToAppend))
350+
for i := range dirsToAppend {
351+
sortedIndices = append(sortedIndices, i)
352+
}
353+
sort.Ints(sortedIndices)
354+
for _, i := range sortedIndices {
355+
dirs := dirsToAppend[i]
356+
entryNode := updatesNode.Content[i]
357+
dirsNode := findMappingValue(entryNode, "directories")
358+
if dirsNode != nil {
359+
newDirs := uniqueStrings(append(cfg.Updates[i].Directories, dirs...))
360+
newLines, netChange, ch := replaceSequence(inputLines, dirsNode, newDirs, lineOffset)
361+
if ch {
362+
inputLines = newLines
363+
lineOffset += netChange
366364
response.IsChanged = true
367365
}
368366
}
369367
}
370368

369+
for _, eco := range newEntries {
370+
item := ecosystemToExtendedUpdate(eco)
371+
items := []ExtendedUpdate{item}
372+
addedItem, err := yaml.Marshal(items)
373+
if err != nil {
374+
return nil, fmt.Errorf("failed to marshal update items: %v", err)
375+
}
376+
data, err := addIndentation(string(addedItem), indentation)
377+
if err != nil {
378+
return nil, fmt.Errorf("failed to add indentation: %v", err)
379+
}
380+
381+
// Trim trailing newline to avoid double blank lines when content
382+
// follows after the updates section (e.g. registries block).
383+
dataLines := strings.Split(strings.TrimRight(data, "\n"), "\n")
384+
insertAt := updatesLastLine + lineOffset
385+
inputLines = insertAfterLine(inputLines, insertAt, dataLines)
386+
lineOffset += len(dataLines)
387+
response.IsChanged = true
388+
}
389+
371390
response.FinalOutput = strings.Join(inputLines, "\n")
372391
if !strings.HasSuffix(response.FinalOutput, "\n") {
373392
response.FinalOutput += "\n"
@@ -402,3 +421,16 @@ func addIndentation(data string, indentation int) (string, error) {
402421

403422
return finalData.String(), nil
404423
}
424+
425+
// uniqueStrings removes duplicate strings from a slice while preserving order.
426+
func uniqueStrings(s []string) []string {
427+
seen := map[string]bool{}
428+
result := make([]string, 0, len(s))
429+
for _, v := range s {
430+
if !seen[v] {
431+
seen[v] = true
432+
result = append(result, v)
433+
}
434+
}
435+
return result
436+
}

‎remediation/dependabot/dependabotconfig_test.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,27 @@ func TestGroups(t *testing.T) {
246246
subtractive: false,
247247
isChanged: true,
248248
},
249+
{
250+
// Additive — three docker directories appended to the same docker entry,
251+
// three npm directories appended to the same npm entry, plus a new pip
252+
// ecosystem that doesn't exist in the config (gets added as new entry).
253+
// Verifies collect-then-apply dedup, sorted index processing across
254+
// multiple entries, and new entry insertion with registries, comments,
255+
// and labels preserved.
256+
inputFileName: "directories-append-multiple.yml",
257+
outputFileName: "directories-append-multiple.yml",
258+
ecosystems: []Ecosystem{
259+
{PackageEcosystem: "docker", Directory: "/api", Interval: "daily"},
260+
{PackageEcosystem: "docker", Directory: "/shared", Interval: "daily"},
261+
{PackageEcosystem: "docker", Directory: "/workers", Interval: "daily"},
262+
{PackageEcosystem: "npm", Directory: "/backend", Interval: "weekly"},
263+
{PackageEcosystem: "npm", Directory: "/api-client", Interval: "weekly"},
264+
{PackageEcosystem: "npm", Directory: "/common", Interval: "weekly"},
265+
{PackageEcosystem: "pip", Directory: "/backend", Interval: "weekly"},
266+
},
267+
subtractive: false,
268+
isChanged: true,
269+
},
249270
}
250271

251272
for _, test := range tests {
@@ -829,6 +850,24 @@ func TestUpdateSubtractiveFields(t *testing.T) {
829850
},
830851
isChanged: true,
831852
},
853+
{
854+
// Subtractive — three docker directories appended to docker entry,
855+
// three npm directories appended to npm entry, plus a new pip ecosystem
856+
// (gets added). Docker interval changed from daily to weekly.
857+
// Verifies merged replaceSequence across multiple entries, new entry
858+
// insertion, and preservation of registries, comments, and labels.
859+
fileName: "subtractive-directories-append-multiple.yml",
860+
ecosystems: []Ecosystem{
861+
{PackageEcosystem: "docker", Directory: "/api", Interval: "weekly"},
862+
{PackageEcosystem: "docker", Directory: "/shared", Interval: "weekly"},
863+
{PackageEcosystem: "docker", Directory: "/workers", Interval: "weekly"},
864+
{PackageEcosystem: "npm", Directory: "/backend", Interval: "daily"},
865+
{PackageEcosystem: "npm", Directory: "/api-client", Interval: "daily"},
866+
{PackageEcosystem: "npm", Directory: "/common", Interval: "daily"},
867+
{PackageEcosystem: "pip", Directory: "/backend", Interval: "weekly"},
868+
},
869+
isChanged: true,
870+
},
832871
{
833872
// Subtractive — update all library-supported fields: scalars (interval,
834873
// rebase-strategy, target-branch, versioning-strategy, milestone,

‎testfiles/dependabotfiles/input/directories-append-multiple.yml‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
version: 2
2+
registries:
3+
docker-hub:
4+
type: docker-registry
5+
url: https://registry.hub.docker.com
6+
username: octocat
7+
password: ${{ secrets.DOCKER_HUB_PASSWORD }}
8+
updates:
9+
# Docker images — scanned across multiple service roots
10+
- package-ecosystem: docker
11+
directories:
12+
- /
13+
- /app
14+
schedule:
15+
interval: daily
16+
labels:
17+
- "docker"
18+
- "dependencies"
19+
# GitHub Actions kept up to date
20+
- package-ecosystem: github-actions
21+
directory: /
22+
schedule:
23+
interval: weekly
24+
# Keep CI actions up to date automatically
25+
labels:
26+
- "ci"
27+
# npm packages across frontend services
28+
- package-ecosystem: npm
29+
directories:
30+
- /frontend
31+
- /admin
32+
- /shared-ui
33+
schedule:
34+
interval: weekly
35+
labels:
36+
- "javascript"

‎testfiles/dependabotfiles/input/subtractive-directories-append-multiple.yml‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
version: 2
2+
registries:
3+
docker-hub:
4+
type: docker-registry
5+
url: https://registry.hub.docker.com
6+
username: octocat
7+
password: ${{ secrets.DOCKER_HUB_PASSWORD }}
8+
updates:
9+
# Docker images — scanned across multiple service roots
10+
- package-ecosystem: docker
11+
directories:
12+
- /
13+
- /app
14+
schedule:
15+
interval: daily
16+
labels:
17+
- "docker"
18+
- "dependencies"
19+
# GitHub Actions kept up to date
20+
- package-ecosystem: github-actions
21+
directory: /
22+
schedule:
23+
interval: weekly
24+
# Keep CI actions up to date automatically
25+
labels:
26+
- "ci"
27+
# npm packages across frontend services
28+
- package-ecosystem: npm
29+
directories:
30+
- /frontend
31+
- /admin
32+
- /shared-ui
33+
schedule:
34+
interval: weekly
35+
labels:
36+
- "javascript"

‎testfiles/dependabotfiles/output/directories-append-multiple.yml‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
version: 2
2+
registries:
3+
docker-hub:
4+
type: docker-registry
5+
url: https://registry.hub.docker.com
6+
username: octocat
7+
password: ${{ secrets.DOCKER_HUB_PASSWORD }}
8+
updates:
9+
# Docker images — scanned across multiple service roots
10+
- package-ecosystem: docker
11+
directories:
12+
- /
13+
- /app
14+
- /api
15+
- /shared
16+
- /workers
17+
schedule:
18+
interval: daily
19+
labels:
20+
- "docker"
21+
- "dependencies"
22+
# GitHub Actions kept up to date
23+
- package-ecosystem: github-actions
24+
directory: /
25+
schedule:
26+
interval: weekly
27+
# Keep CI actions up to date automatically
28+
labels:
29+
- "ci"
30+
# npm packages across frontend services
31+
- package-ecosystem: npm
32+
directories:
33+
- /frontend
34+
- /admin
35+
- /shared-ui
36+
- /backend
37+
- /api-client
38+
- /common
39+
schedule:
40+
interval: weekly
41+
labels:
42+
- "javascript"
43+
44+
- package-ecosystem: pip
45+
directory: /backend
46+
schedule:
47+
interval: weekly

‎testfiles/dependabotfiles/output/subtractive-directories-append-multiple.yml‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
version: 2
2+
registries:
3+
docker-hub:
4+
type: docker-registry
5+
url: https://registry.hub.docker.com
6+
username: octocat
7+
password: ${{ secrets.DOCKER_HUB_PASSWORD }}
8+
updates:
9+
# Docker images — scanned across multiple service roots
10+
- package-ecosystem: docker
11+
directories:
12+
- /
13+
- /app
14+
- /api
15+
- /shared
16+
- /workers
17+
schedule:
18+
interval: weekly
19+
labels:
20+
- "docker"
21+
- "dependencies"
22+
# GitHub Actions kept up to date
23+
- package-ecosystem: github-actions
24+
directory: /
25+
schedule:
26+
interval: weekly
27+
# Keep CI actions up to date automatically
28+
labels:
29+
- "ci"
30+
# npm packages across frontend services
31+
- package-ecosystem: npm
32+
directories:
33+
- /frontend
34+
- /admin
35+
- /shared-ui
36+
- /backend
37+
- /api-client
38+
- /common
39+
schedule:
40+
interval: daily
41+
labels:
42+
- "javascript"
43+
44+
- package-ecosystem: pip
45+
directory: /backend
46+
schedule:
47+
interval: weekly

0 commit comments

Comments
 (0)