Skip to content

Commit e4209db

Browse files
committed
Fix SIGSEGV when resolving charts dependencies
If implemented, this make sure than we clear only referenced downloaders. It is also checked if the repository url is supported. Signed-off-by: Soule BA <[email protected]>
1 parent 7b4ba69 commit e4209db

File tree

3 files changed

+44
-1
lines changed

3 files changed

+44
-1
lines changed

internal/helm/chart/dependency_manager.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,9 @@ func NewDependencyManager(opts ...DependencyManagerOption) *DependencyManager {
9898
func (dm *DependencyManager) Clear() error {
9999
var errs []error
100100
for _, v := range dm.downloaders {
101-
errs = append(errs, v.Clear())
101+
if v != nil {
102+
errs = append(errs, v.Clear())
103+
}
102104
}
103105
return errors.NewAggregate(errs)
104106
}
@@ -257,6 +259,10 @@ func (dm *DependencyManager) resolveRepository(url string) (repo repository.Down
257259
defer dm.mu.Unlock()
258260

259261
nUrl := repository.NormalizeURL(url)
262+
err = repository.ValidateDepURL(nUrl)
263+
if err != nil {
264+
return
265+
}
260266
if _, ok := dm.downloaders[nUrl]; !ok {
261267
if dm.getChartDownloaderCallback == nil {
262268
err = fmt.Errorf("no chart repository for URL '%s'", nUrl)

internal/helm/chart/dependency_manager_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ func TestDependencyManager_Clear(t *testing.T) {
9393
},
9494
"with credentials": ociRepoWithCreds,
9595
"without credentials": &repository.OCIChartRepository{},
96+
"nil downloader": nil,
9697
}
9798

9899
dm := NewDependencyManager(WithRepositories(downloaders))
@@ -428,6 +429,14 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) {
428429
},
429430
wantErr: "no chart repository for URL",
430431
},
432+
{
433+
name: "resolve aliased repository error",
434+
downloaders: map[string]repository.Downloader{},
435+
dep: &helmchart.Dependency{
436+
Repository: "@fantastic-charts",
437+
},
438+
wantErr: "aliased repository dependency is not supported",
439+
},
431440
{
432441
name: "strategic load error",
433442
downloaders: map[string]repository.Downloader{

internal/helm/repository/utils.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,23 @@ limitations under the License.
1717
package repository
1818

1919
import (
20+
"fmt"
2021
"strings"
2122

2223
helmreg "helm.sh/helm/v3/pkg/registry"
2324
)
2425

26+
const (
27+
alias = "@"
28+
)
29+
30+
var (
31+
// errInvalidDepURL is returned when the dependency URL is not supported
32+
errInvalidDepURL = fmt.Errorf("invalid dependency repository URL")
33+
// errInvalidAliasedDep is returned when the dependency URL is an alias
34+
errInvalidAliasedDep = fmt.Errorf("aliased repository dependency is not supported")
35+
)
36+
2537
// NormalizeURL normalizes a ChartRepository URL by its scheme.
2638
func NormalizeURL(repositoryURL string) string {
2739
if repositoryURL == "" {
@@ -35,3 +47,19 @@ func NormalizeURL(repositoryURL string) string {
3547
return strings.TrimRight(repositoryURL, "/") + "/"
3648

3749
}
50+
51+
// ValidateDepURL returns an error if the given depended repository URL declaration is not supported
52+
// The reason for this is that the dependency manager will not be able to resolve the alias declaration
53+
// e.g. repository: "@fantastic-charts"
54+
func ValidateDepURL(repositoryURL string) error {
55+
switch {
56+
case strings.HasPrefix(repositoryURL, helmreg.OCIScheme):
57+
return nil
58+
case strings.HasPrefix(repositoryURL, "https://") || strings.HasPrefix(repositoryURL, "http://"):
59+
return nil
60+
case strings.HasPrefix(repositoryURL, alias):
61+
return fmt.Errorf("%w: %s", errInvalidAliasedDep, repositoryURL)
62+
default:
63+
return fmt.Errorf("%w: %s", errInvalidDepURL, repositoryURL)
64+
}
65+
}

0 commit comments

Comments
 (0)