diff --git a/configmerge/configmerge.go b/configmerge/configmerge.go index 7cb55bf4..9f46f879 100644 --- a/configmerge/configmerge.go +++ b/configmerge/configmerge.go @@ -14,8 +14,9 @@ import ( ) const ( - MaxFilesCountTotal = 20 - MaxIncludeDepth = 5 // root + 4 includes + MaxIncludeCountPerFile = 10 + MaxFilesCountTotal = 20 + MaxIncludeDepth = 5 // root + 4 includes ) func IsModularConfig(mainConfigPth string) (bool, error) { @@ -54,7 +55,6 @@ type Merger struct { repoInfo RepoInfo filesCount int - keys []string } func NewMerger(repoInfoProvider RepoInfoProvider, fileReader FileReader, logger logV2.Logger) Merger { @@ -86,7 +86,7 @@ func (m *Merger) MergeConfig(mainConfigPth string) (string, *models.ConfigFileTr return "", nil, err } - configTree, err := m.buildConfigTree(mainConfigBytes, mainConfigRef, 1) + configTree, err := m.buildConfigTree(mainConfigBytes, mainConfigRef, 1, nil) if err != nil { return "", nil, err } @@ -99,16 +99,13 @@ func (m *Merger) MergeConfig(mainConfigPth string) (string, *models.ConfigFileTr return mergedConfigContent, configTree, nil } -func (m *Merger) buildConfigTree(configContent []byte, reference ConfigReference, depth int) (*models.ConfigFileTreeModel, error) { +func (m *Merger) buildConfigTree(configContent []byte, reference ConfigReference, depth int, keys []string) (*models.ConfigFileTreeModel, error) { + keys = append(keys, reference.Key()) + if depth > MaxIncludeDepth { return nil, fmt.Errorf("max include depth (%d) exceeded", MaxIncludeDepth) } - if sliceutil.IsStringInSlice(reference.Key(), m.keys) { - return nil, fmt.Errorf("circular includes detected: %s -> %s", strings.Join(m.keys, " -> "), reference.Key()) - } - m.keys = append(m.keys, reference.Key()) - m.filesCount++ if m.filesCount > MaxFilesCountTotal { return nil, fmt.Errorf("max include count (%d) exceeded", MaxFilesCountTotal) @@ -131,10 +128,19 @@ func (m *Merger) buildConfigTree(configContent []byte, reference ConfigReference include.Tag = reference.Tag } config.Include[idx] = include + + if sliceutil.IsStringInSlice(include.Key(), keys) { + return nil, fmt.Errorf("circular reference detected: %s -> %s", strings.Join(keys, " -> "), include.Key()) + } + } + + if len(config.Include) > MaxIncludeCountPerFile { + return nil, fmt.Errorf("max include count (%d) exceeded", MaxIncludeCountPerFile) + } if m.filesCount+len(config.Include) > MaxFilesCountTotal { - return nil, fmt.Errorf("max include count (%d) exceeded", MaxFilesCountTotal) + return nil, fmt.Errorf("max file count (%d) exceeded", MaxFilesCountTotal) } var includedConfigTrees []models.ConfigFileTreeModel @@ -144,7 +150,7 @@ func (m *Merger) buildConfigTree(configContent []byte, reference ConfigReference return nil, err } - moduleConfigTree, err := m.buildConfigTree(moduleBytes, include, depth+1) + moduleConfigTree, err := m.buildConfigTree(moduleBytes, include, depth+1, keys) if err != nil { return nil, err } diff --git a/configmerge/configmerge_test.go b/configmerge/configmerge_test.go index 7d9ad1db..cd73aaac 100644 --- a/configmerge/configmerge_test.go +++ b/configmerge/configmerge_test.go @@ -43,6 +43,28 @@ include: mainConfigPth: "bitrise.yml", wantErr: "circular includes detected: repo:https://github.com/bitrise-io/example.git,bitrise.yml@commit:016883ca9498f75d03cd45c0fa400ad9f8141edf -> repo:https://github.com/bitrise-io/example.git,module_1.yml@commit:016883ca9498f75d03cd45c0fa400ad9f8141edf -> repo:https://github.com/bitrise-io/example.git,module_2.yml@commit:016883ca9498f75d03cd45c0fa400ad9f8141edf -> repo:https://github.com/bitrise-io/example.git,module_1.yml@commit:016883ca9498f75d03cd45c0fa400ad9f8141edf", }, + { + name: "Max 10 include items are allowed", + repoInfoProvider: mockRepoInfoProvider{ + repoInfo: &RepoInfo{ + DefaultRemoteURL: "https://github.com/bitrise-io/example.git", + Branch: "main", + Commit: "016883ca9498f75d03cd45c0fa400ad9f8141edf", + }, + err: nil, + }, + fileReader: mockFileReader{ + fileSystemFiles: map[string][]byte{ + "bitrise.yml": []byte(fmt.Sprintf(`format_version: "15" +default_step_lib_source: https://github.com/bitrise-io/bitrise-steplib.git + +include: +%s`, strings.Repeat("- path: path_1.yml\n", MaxIncludeCountPerFile+1))), + }, + }, + mainConfigPth: "bitrise.yml", + wantErr: "max include count (10) exceeded", + }, { name: "Max 20 config files are allowed", repoInfoProvider: mockRepoInfoProvider{ @@ -59,11 +81,13 @@ include: default_step_lib_source: https://github.com/bitrise-io/bitrise-steplib.git include: -%s`, strings.Repeat("- path: path_1.yml\n", MaxFilesCountTotal))), +%s`, strings.Repeat("- path: path_1.yml\n", 10))), + "path_1.yml": []byte(`include: +- path: path_2.yml`), }, }, mainConfigPth: "bitrise.yml", - wantErr: "max include count (20) exceeded", + wantErr: "max file count (20) exceeded", }, { name: "Max include depth is 5",