refactor(misconf): replace custom Helm archive parsing with Helm SDK loaders (#10718)

Author: nikpivkin

internal/testutil/gzip.go

@@ -1,9 +1,11 @@
 package testutil
 
 import (
+	"archive/tar"
 	"bytes"
 	"compress/gzip"
 	"io"
+	"io/fs"
 	"os"
 	"testing"
 
@@ -75,3 +77,39 @@ func DecompressSparseGzip(t *testing.T, src, dst string) {
 		}
 	}
 }
+
+// FSToTarGz creates an in-memory tar.gz archive from an fs.FS.
+// prefix is prepended to every file path inside the archive.
+func FSToTarGz(t *testing.T, fsys fs.FS, prefix string) []byte {
+	t.Helper()
+	var buf bytes.Buffer
+	gw := gzip.NewWriter(&buf)
+	tw := tar.NewWriter(gw)
+	require.NoError(t, writeFSToTar(tw, fsys, prefix))
+	require.NoError(t, tw.Close())
+	require.NoError(t, gw.Close())
+	return buf.Bytes()
+}
+
+func writeFSToTar(tw *tar.Writer, fsys fs.FS, prefix string) error {
+	return fs.WalkDir(fsys, ".", func(path string, d fs.DirEntry, err error) error {
+		if err != nil || d.IsDir() {
+			return err
+		}
+		data, err := fs.ReadFile(fsys, path)
+		if err != nil {
+			return err
+		}
+		name := path
+		if prefix != "" {
+			name = prefix + "/" + path
+		}
+		if err := tw.WriteHeader(&tar.Header{
+			Name: name, Typeflag: tar.TypeReg, Size: int64(len(data)), Mode: 0o644,
+		}); err != nil {
+			return err
+		}
+		_, err = tw.Write(data)
+		return err
+	})
+}

pkg/iac/detection/peek.go

@@ -45,7 +45,7 @@ func IsHelmChartArchive(path string, file io.Reader) bool {
 }
 
 func IsArchive(path string) bool {
-	return strings.HasSuffix(path, ".tar") || IsZip(path)
+	return IsZip(path)
 }
 
 func IsZip(path string) bool {

pkg/iac/scanners/helm/parser/parser.go

@@ -19,22 +19,18 @@ import (
 	"helm.sh/helm/v4/pkg/chart/loader/archive"
 	chartv2 "helm.sh/helm/v4/pkg/chart/v2"
 	loaderv2 "helm.sh/helm/v4/pkg/chart/v2/loader"
+	chartutilv2 "helm.sh/helm/v4/pkg/chart/v2/util"
 	"helm.sh/helm/v4/pkg/release"
 	releaseutil "helm.sh/helm/v4/pkg/release/v1/util"
 
-	"github.com/aquasecurity/trivy/pkg/iac/detection"
 	"github.com/aquasecurity/trivy/pkg/log"
-	"github.com/aquasecurity/trivy/pkg/mapfs"
 )
 
 var manifestNameRegex = regexp.MustCompile("# Source: [^/]+/(.+)")
 
 type Parser struct {
 	logger       *log.Logger
 	helmClient   *action.Install
-	rootPath     string
-	ChartSource  string
-	filepaths    map[string]fs.FS
 	valuesFiles  []string
 	values       []string
 	fileValues   []string
@@ -43,22 +39,20 @@ type Parser struct {
 	kubeVersion  string
 }
 
-type ChartFile struct {
-	TemplateFilePath string
-	ManifestContent  string
+// Manifest is a rendered Helm template — the output of helm template for a single Kubernetes resource.
+type Manifest struct {
+	Path    string
+	Content string
 }
 
-func New(src string, opts ...Option) (*Parser, error) {
-
+func New(opts ...Option) (*Parser, error) {
 	client := action.NewInstall(&action.Configuration{})
 	client.DryRunStrategy = action.DryRunClient // to avoid the client making calls to the server
 	client.Replace = true                       // skip name check
 
 	p := &Parser{
-		helmClient:  client,
-		ChartSource: src,
-		logger:      log.WithPrefix("helm parser"),
-		filepaths:   make(map[string]fs.FS),
+		helmClient: client,
+		logger:     log.WithPrefix("helm parser"),
 	}
 
 	for _, option := range opts {
@@ -81,11 +75,11 @@ func New(src string, opts ...Option) (*Parser, error) {
 	return p, nil
 }
 
-func (p *Parser) ParseFS(ctx context.Context, fsys fs.FS, target string) error {
-	return p.parseFS(ctx, fsys, target)
-}
+// ParseFS renders a Helm chart from a directory rooted at target within fsys.
+func (p *Parser) ParseFS(ctx context.Context, fsys fs.FS, target string) ([]Manifest, error) {
+	var rootPath string
+	filepaths := make(map[string]fs.FS)
 
-func (p *Parser) parseFS(ctx context.Context, fsys fs.FS, target string) error {
 	target = filepath.ToSlash(target)
 	if err := fs.WalkDir(fsys, target, func(filePath string, entry fs.DirEntry, err error) error {
 		select {
@@ -100,102 +94,123 @@ func (p *Parser) parseFS(ctx context.Context, fsys fs.FS, target string) error {
 			return nil
 		}
 
-		if detection.IsArchive(filePath) && !isDependencyChartArchive(fsys, filePath) {
-			memFS := mapfs.New()
-			if err := p.unpackArchive(fsys, memFS, filePath); errors.Is(err, errSkipFS) {
-				// an unpacked Chart already exists
-				return nil
-			} else if err != nil {
-				return fmt.Errorf("unpack archive %q: %w", filePath, err)
+		if strings.HasSuffix(filePath, chartutilv2.ChartfileName) && rootPath == "" {
+			data, err := fs.ReadFile(fsys, filePath)
+			if err != nil {
+				return err
 			}
-
-			if err := p.parseFS(ctx, memFS, "."); err != nil {
-				return fmt.Errorf("parse archive FS error: %w", err)
+			dir := path.Dir(filePath)
+			if dir == "." {
+				dir = ""
 			}
-			return nil
+			p.applyChartName(data, dir)
+			rootPath = path.Dir(filePath)
 		}
-
-		return p.addPaths(fsys, filePath)
+		filepaths[filePath] = fsys
+		return nil
 	}); err != nil {
-		return fmt.Errorf("walk dir error: %w", err)
+		return nil, fmt.Errorf("walk dir error: %w", err)
 	}
 
-	return nil
-}
-
-func isDependencyChartArchive(fsys fs.FS, archivePath string) bool {
-	parent := path.Dir(archivePath)
-	if path.Base(parent) != "charts" {
-		return false
+	var files []*archive.BufferedFile
+	for filePath, fsys := range filepaths {
+		b, err := fs.ReadFile(fsys, filePath)
+		if err != nil {
+			return nil, err
+		}
+		filePath = strings.TrimPrefix(filePath, rootPath+"/")
+		filePath = filepath.ToSlash(filePath)
+		files = append(files, &archive.BufferedFile{
+			Name: filePath,
+			Data: b,
+		})
 	}
 
-	_, err := fs.Stat(fsys, path.Join(parent, "..", "Chart.yaml"))
-	return err == nil
+	return p.render(ctx, files)
 }
 
-func (p *Parser) addPaths(fsys fs.FS, paths ...string) error {
-	for _, filePath := range paths {
-		if _, err := fs.Stat(fsys, filePath); err != nil {
-			return err
-		}
+// ParseArchive renders a Helm chart from a gzip-compressed tar archive within fsys.
+func (p *Parser) ParseArchive(ctx context.Context, fsys fs.FS, archivePath string) ([]Manifest, error) {
+	f, err := fsys.Open(archivePath)
+	if err != nil {
+		return nil, fmt.Errorf("open archive: %w", err)
+	}
+	defer f.Close()
 
-		if strings.HasSuffix(filePath, "Chart.yaml") && p.rootPath == "" {
-			if err := p.extractChartName(fsys, filePath); err != nil {
-				return err
-			}
-			p.rootPath = filepath.Dir(filePath)
+	files, err := archive.LoadArchiveFiles(f)
+	if err != nil {
+		return nil, fmt.Errorf("load archive files: %w", err)
+	}
+
+	for _, file := range files {
+		if file.Name == chartutilv2.ChartfileName {
+			p.applyChartName(file.Data, "")
+			break
 		}
-		p.filepaths[filePath] = fsys
 	}
-	return nil
+
+	return p.render(ctx, files)
 }
 
-func (p *Parser) extractChartName(fsys fs.FS, chartPath string) error {
-	chrt, err := fsys.Open(chartPath)
-	if err != nil {
-		return err
+func (p *Parser) applyChartName(data []byte, fallback string) {
+	if name, err := parseChartName(data); err == nil {
+		p.helmClient.ReleaseName = name
+		return
 	}
-	defer func() { _ = chrt.Close() }()
-
-	var chartContent map[string]any
-	if err := yaml.NewDecoder(chrt).Decode(&chartContent); err != nil {
-		// the chart likely has the name templated and so cannot be parsed as yaml - use a temporary name
-		if dir := filepath.Dir(chartPath); dir != "" && dir != "." {
-			p.helmClient.ReleaseName = dir
-		} else {
-			p.helmClient.ReleaseName = uuid.NewString()
-		}
-		return nil
+	if fallback != "" {
+		p.helmClient.ReleaseName = fallback
+	} else {
+		p.helmClient.ReleaseName = uuid.NewString()
 	}
+}
 
-	name, ok := chartContent["name"]
-	if !ok {
-		return fmt.Errorf("could not extract the chart name from %s", chartPath)
+func parseChartName(data []byte) (string, error) {
+	var meta struct {
+		Name string `yaml:"name"`
 	}
-	p.helmClient.ReleaseName = fmt.Sprintf("%v", name)
-	return nil
+	if err := yaml.Unmarshal(data, &meta); err != nil {
+		return "", err
+	}
+	if meta.Name == "" {
+		return "", errors.New("could not extract the chart name from Chart.yaml")
+	}
+	return meta.Name, nil
 }
 
-func (p *Parser) RenderedChartFiles() ([]ChartFile, error) {
-	chrt, err := p.loadChart()
+func (p *Parser) render(ctx context.Context, files []*archive.BufferedFile) ([]Manifest, error) {
+	chrt, err := p.loadChart(files)
 	if err != nil {
 		return nil, err
 	}
 
-	acc, err := p.getRelease(chrt)
+	acc, err := p.getRelease(ctx, chrt)
 	if err != nil {
 		return nil, err
 	}
 
-	splitManifests := releaseutil.SplitManifests(strings.TrimSpace(acc.Manifest()))
-	manifestsKeys := make([]string, 0, len(splitManifests))
-	for k := range splitManifests {
-		manifestsKeys = append(manifestsKeys, k)
+	manifests := releaseutil.SplitManifests(strings.TrimSpace(acc.Manifest()))
+	keys := make([]string, 0, len(manifests))
+	for k := range manifests {
+		keys = append(keys, k)
 	}
-	return p.getRenderedManifests(manifestsKeys, splitManifests), nil
+	sort.Sort(releaseutil.BySplitManifestsOrder(keys))
+
+	var result []Manifest
+	for _, key := range keys {
+		manifest := manifests[key]
+		submatch := manifestNameRegex.FindStringSubmatch(manifest)
+		if submatch == nil {
+			continue
+		}
+		result = append(result, Manifest{
+			Path:    submatch[1],
+			Content: manifest,
+		})
+	}
+	return result, nil
 }
 
-func (p *Parser) getRelease(chrt *chartv2.Chart) (release.Accessor, error) {
+func (p *Parser) getRelease(ctx context.Context, chrt *chartv2.Chart) (release.Accessor, error) {
 	opts := &ValueOptions{
 		ValueFiles:   p.valuesFiles,
 		Values:       p.values,
@@ -208,7 +223,7 @@ func (p *Parser) getRelease(chrt *chartv2.Chart) (release.Accessor, error) {
 		return nil, err
 	}
 
-	rel, err := p.helmClient.RunWithContext(context.Background(), chrt, vals)
+	rel, err := p.helmClient.RunWithContext(ctx, chrt, vals)
 	if err != nil {
 		return nil, err
 	}
@@ -224,23 +239,7 @@ func (p *Parser) getRelease(chrt *chartv2.Chart) (release.Accessor, error) {
 	return acc, nil
 }
 
-func (p *Parser) loadChart() (*chartv2.Chart, error) {
-	var files []*archive.BufferedFile
-
-	for filePath, fsys := range p.filepaths {
-		b, err := fs.ReadFile(fsys, filePath)
-		if err != nil {
-			return nil, err
-		}
-
-		filePath = strings.TrimPrefix(filePath, p.rootPath+"/")
-		filePath = filepath.ToSlash(filePath)
-		files = append(files, &archive.BufferedFile{
-			Name: filePath,
-			Data: b,
-		})
-	}
-
+func (p *Parser) loadChart(files []*archive.BufferedFile) (*chartv2.Chart, error) {
 	chrt, err := loaderv2.LoadFiles(files)
 	if err != nil {
 		return nil, err
@@ -259,32 +258,3 @@ func (p *Parser) loadChart() (*chartv2.Chart, error) {
 
 	return chrt, nil
 }
-
-func (*Parser) getRenderedManifests(manifestsKeys []string, splitManifests map[string]string) []ChartFile {
-	sort.Sort(releaseutil.BySplitManifestsOrder(manifestsKeys))
-	var manifestsToRender []ChartFile
-	for _, manifestKey := range manifestsKeys {
-		manifest := splitManifests[manifestKey]
-		submatch := manifestNameRegex.FindStringSubmatch(manifest)
-		if len(submatch) == 0 {
-			continue
-		}
-		manifestsToRender = append(manifestsToRender, ChartFile{
-			TemplateFilePath: getManifestPath(manifest),
-			ManifestContent:  manifest,
-		})
-	}
-	return manifestsToRender
-}
-
-func getManifestPath(manifest string) string {
-	lines := strings.Split(manifest, "\n")
-	if len(lines) == 0 {
-		return "unknown.yaml"
-	}
-	manifestFilePathParts := strings.SplitN(strings.TrimPrefix(lines[0], "# Source: "), "/", 2)
-	if len(manifestFilePathParts) > 1 {
-		return manifestFilePathParts[1]
-	}
-	return manifestFilePathParts[0]
-}