From 9d222579b4bbc184d425ff038cd6c09e6d0f0280 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Fri, 24 Apr 2026 13:13:42 +0200 Subject: [PATCH 01/17] Extend the model with Kustomize kind, virtual resolved files, resolver diagnostics, and provenance types. --- pkg/model/model.go | 72 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 68 insertions(+), 4 deletions(-) diff --git a/pkg/model/model.go b/pkg/model/model.go index fe9a1c298..177c5fdde 100644 --- a/pkg/model/model.go +++ b/pkg/model/model.go @@ -26,6 +26,7 @@ const ( KindPROTO FileKind = "PROTO" KindCOMMON FileKind = "*" KindHELM FileKind = "HELM" + KindKUSTOMIZE FileKind = "KUSTOMIZE" KindBUILDAH FileKind = "SH" KindCFG FileKind = "CFG" KindINI FileKind = "INI" @@ -151,6 +152,7 @@ type FileMetadata struct { ResolvedFiles map[string]ResolvedFile LinesOriginalData *[]string IsMinified bool + KustomizeOrigin *KustomizeOrigin } // QueryMetadata is a representation of general information about a query @@ -227,17 +229,79 @@ type QueryConfig struct { // ResolvedFiles keeps the information of all file/template resolved type ResolvedFiles struct { - File []ResolvedHelm - Excluded []string + File []ResolvedVirtual + Excluded []string + Diagnostics []ResolverDiagnostic } -// ResolvedHelm keeps the information of a file/template resolved -type ResolvedHelm struct { +// ResolverDiagnostic is a preprocessor-issued finding (not from Rego). +type ResolverDiagnostic struct { + FilePath string + Message string + QueryID string + Line int +} + +// ResolvedVirtual is one rendered virtual file from a preprocessor (Helm, Kustomize, …). +type ResolvedVirtual struct { FileName string + MetadataPath string Content []byte OriginalData []byte SplitID string IDInfo map[int]interface{} + Origin *KustomizeOrigin +} + +// ResolvedHelm is a legacy alias for ResolvedVirtual. +type ResolvedHelm = ResolvedVirtual + +// KustomizeTransformation records a transformer step for provenance. +type KustomizeTransformation struct { + TransformerPath string + ConfiguredIn string + FieldPath string +} + +// KustomizeOrigin is provenance for a Kustomize-built resource. +type KustomizeOrigin struct { + OriginKind KustomizeOriginKind + SourceFile string + SourceRepo string + SourceRef string + OriginalSourceFile string + OriginalSourceRepo string + OriginalSourceRef string + GeneratorKind string + ConfiguredByKind string + ConfiguredByName string + GeneratorConfigFile string + Transformations []KustomizeTransformation + ResourceGVK string + ResourceName string +} + +// KustomizeOriginKind classifies provenance back to source. +type KustomizeOriginKind string + +const ( + KustomizeOriginDirect KustomizeOriginKind = "direct" + KustomizeOriginGenerator KustomizeOriginKind = "generator" + KustomizeOriginTransformer KustomizeOriginKind = "transformer" + KustomizeOriginHelmInflated KustomizeOriginKind = "helm_inflated" +) + +// RequiresDetailedLineMapping skips the searchLine fast path (detector maps lines). +func (o *KustomizeOrigin) RequiresDetailedLineMapping() bool { + if o == nil { + return false + } + switch o.OriginKind { + case KustomizeOriginGenerator, KustomizeOriginTransformer: + return true + default: + return false + } } // Extensions represents a list of supported extensions From 2305013a7a73eec7f32a46f9de0d48d73097d3da Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Fri, 24 Apr 2026 13:14:03 +0200 Subject: [PATCH 02/17] Add the Preprocessor API with ordered directory detection and refactor Helm dry-run rendering to match it. --- pkg/resolver/helm/helm.go | 57 +++-- pkg/resolver/helm/render.go | 339 +++++++++++++++++++++++++++++ pkg/resolver/helm/render_test.go | 148 +++++++++++++ pkg/resolver/helm/resolver.go | 209 +++++------------- pkg/resolver/helm/resolver_test.go | 24 +- pkg/resolver/resolver.go | 60 ++--- 6 files changed, 634 insertions(+), 203 deletions(-) create mode 100644 pkg/resolver/helm/render.go create mode 100644 pkg/resolver/helm/render_test.go diff --git a/pkg/resolver/helm/helm.go b/pkg/resolver/helm/helm.go index 6c8e3ecfb..e729d75af 100644 --- a/pkg/resolver/helm/helm.go +++ b/pkg/resolver/helm/helm.go @@ -23,12 +23,8 @@ import ( // credit: https://github.com/helm/helm -var ( - settings = cli.New() -) - func runInstall(ctx context.Context, args []string, client *action.Install, - valueOpts *values.Options) (*release.Release, []string, error) { + valueOpts *values.Options, opts *RenderOptions) (*release.Release, []string, error) { contextLogger := logger.FromContext(ctx) log.SetOutput(io.Discard) defer log.SetOutput(os.Stderr) @@ -46,16 +42,28 @@ func runInstall(ctx context.Context, args []string, client *action.Install, return nil, []string{}, err } contextLogger.Debug().Msgf("Parsed chart name: '%s', chart path: '%s'", name, charts) - client.ReleaseName = name + if strings.TrimSpace(client.ReleaseName) == "" || client.GenerateName { + client.ReleaseName = name + } - cp, err := client.LocateChart(charts, settings) + helmSettings := cli.New() + if strings.TrimSpace(opts.RepositoryConfig) != "" { + helmSettings.RepositoryConfig = opts.RepositoryConfig + } + if strings.TrimSpace(opts.RepositoryCache) != "" { + helmSettings.RepositoryCache = opts.RepositoryCache + } + if strings.TrimSpace(opts.RegistryConfig) != "" { + helmSettings.RegistryConfig = opts.RegistryConfig + } + cp, err := client.LocateChart(charts, helmSettings) if err != nil { contextLogger.Error().Msgf("failed to locate chart '%s': %s", charts, err) return nil, []string{}, err } contextLogger.Debug().Msgf("Located chart at path: '%s'", cp) - p := getter.All(settings) + p := getter.All(helmSettings) vals, err := valueOpts.MergeValues(p) if err != nil { contextLogger.Error().Msgf("failed to merge helm values: %s", err) @@ -81,7 +89,6 @@ func runInstall(ctx context.Context, args []string, client *action.Install, } contextLogger.Debug().Msg("Chart installability check passed") - client.Namespace = "kics-namespace" contextLogger.Debug().Msgf("Running helm chart with namespace: '%s', release name: '%s'", client.Namespace, client.ReleaseName) helmRelease, err := client.Run(chartRequested, vals) if err != nil { @@ -105,8 +112,8 @@ func checkIfInstallable(ch *chart.Chart) error { return errors.Errorf("%s charts are not installable (only 'application' type charts are supported)", ch.Metadata.Type) } -// newClient will create a new instance on helm client used to render the chart -func newClient(ctx context.Context) *action.Install { +// newClient creates a Helm install client used to render the chart (dry-run). +func newClient(ctx context.Context, opts *RenderOptions) *action.Install { contextLogger := logger.FromContext(ctx) contextLogger.Debug().Msg("Creating new helm client for chart rendering") @@ -114,13 +121,33 @@ func newClient(ctx context.Context) *action.Install { client := action.NewInstall(cfg) client.DryRun = true client.ReleaseName = "kics-helm" + if strings.TrimSpace(opts.ReleaseName) != "" { + client.ReleaseName = opts.ReleaseName + } + client.GenerateName = strings.TrimSpace(opts.ReleaseName) == "" client.Replace = true // Skip the name check client.ClientOnly = true - client.APIVersions = chartutil.VersionSet([]string{}) - client.IncludeCRDs = false + client.APIVersions = chartutil.VersionSet(opts.APIVersions) + client.IncludeCRDs = opts.IncludeCRDs + client.DisableHooks = opts.SkipHooks + client.NameTemplate = strings.TrimSpace(opts.NameTemplate) + client.RepoURL = strings.TrimSpace(opts.ChartRepo) + client.Version = strings.TrimSpace(opts.Version) + client.Devel = opts.Devel + if strings.TrimSpace(opts.KubeVersion) != "" { + if kv, err := chartutil.ParseKubeVersion(strings.TrimSpace(opts.KubeVersion)); err == nil { + client.KubeVersion = kv + } + } + if strings.TrimSpace(opts.Namespace) != "" { + client.Namespace = opts.Namespace + } else { + client.Namespace = "kics-namespace" + } - contextLogger.Debug().Msgf("Configured helm client - DryRun: %t, ClientOnly: %t, IncludeCRDs: %t, ReleaseName: '%s'", - client.DryRun, client.ClientOnly, client.IncludeCRDs, client.ReleaseName) + contextLogger.Debug(). + Msgf("Configured helm client - DryRun: %t, ClientOnly: %t, IncludeCRDs: %t, ReleaseName: '%s', Namespace: '%s', DisableHooks: %t", + client.DryRun, client.ClientOnly, client.IncludeCRDs, client.ReleaseName, client.Namespace, client.DisableHooks) return client } diff --git a/pkg/resolver/helm/render.go b/pkg/resolver/helm/render.go new file mode 100644 index 000000000..2b72317d9 --- /dev/null +++ b/pkg/resolver/helm/render.go @@ -0,0 +1,339 @@ +package helm + +import ( + "context" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/DataDog/datadog-iac-scanner/pkg/logger" + "github.com/pkg/errors" + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/cli/values" + "helm.sh/helm/v3/pkg/release" + kyaml "sigs.k8s.io/kustomize/kyaml/yaml" + "sigs.k8s.io/kustomize/kyaml/yaml/merge2" +) + +const ( + kicsHelmID = "# KICS_HELM_ID_" + + valuesMergeMerge = "merge" + valuesMergeOverride = "override" + valuesMergeReplace = "replace" +) + +// splitManifest holds one manifest slice split by source. +type splitManifest struct { + path string + content []byte + original []byte + splitID string + splitIDMap map[int]interface{} +} + +// RenderOptions configures Helm chart rendering (dry-run template). +type RenderOptions struct { + ChartPath string + ChartRepo string + ReleaseName string + NameTemplate string + Namespace string + ValuesFiles []string + SetValues []string + ValuesInline map[string]interface{} + ValuesMerge string + IncludeCRDs bool + SkipHooks bool + SkipTests bool + APIVersions []string + KubeVersion string + Version string + Devel bool + RepositoryConfig string + RepositoryCache string + RegistryConfig string +} + +// RenderedChart is the output of RenderChart. +type RenderedChart struct { + Resources []RenderedResource + Excluded []string +} + +// RenderedResource is one manifest document from a rendered chart. +type RenderedResource struct { + SourceFile string + Content []byte + Original []byte + SplitID string + IDInfo map[int]interface{} +} + +// RenderChart renders a chart at ChartPath using the Helm SDK (dry-run). +func RenderChart(ctx context.Context, opts *RenderOptions) (*RenderedChart, error) { + if opts == nil { + return nil, errors.New("helm RenderOptions is nil") + } + contextLogger := logger.FromContext(ctx) + client := newClient(ctx, opts) + valueOpts := &values.Options{ + ValueFiles: opts.ValuesFiles, + Values: opts.SetValues, + } + cleanup := func() {} + if len(opts.ValuesInline) > 0 { + var err error + cleanup, err = applyValuesInline(ctx, valueOpts, opts) + if err != nil { + return nil, err + } + } + defer cleanup() + manifest, excluded, err := runInstall(ctx, []string{opts.ChartPath}, client, valueOpts, opts) + if err != nil { + contextLogger.Error().Msgf("failed to run helm install for '%s': %s", opts.ChartPath, err) + return nil, errors.Wrap(err, "helm render") + } + splits, err := splitManifestYAML(manifest) + if err != nil { + return nil, err + } + out := &RenderedChart{Excluded: excluded} + for _, sp := range *splits { + if opts.SkipTests && isHelmTestTemplate(sp.path) { + continue + } + out.Resources = append(out.Resources, RenderedResource{ + SourceFile: sp.path, + Content: sp.content, + Original: sp.original, + SplitID: sp.splitID, + IDInfo: sp.splitIDMap, + }) + } + return out, nil +} + +func applyValuesInline(ctx context.Context, valueOpts *values.Options, opts *RenderOptions) (func(), error) { + if opts == nil { + return func() {}, errors.New("helm RenderOptions is nil") + } + contextLogger := logger.FromContext(ctx) + inlineBytes, err := mergedValuesInlineBytes(opts) + if err != nil { + return func() {}, err + } + if len(inlineBytes) == 0 { + return func() {}, nil + } + f, err := os.CreateTemp("", "iac-scanner-helm-values-*.yaml") + if err != nil { + return func() {}, errors.Wrap(err, "create helm values temp file") + } + if _, err := f.Write(inlineBytes); err != nil { + _ = f.Close() + _ = os.Remove(f.Name()) + return func() {}, errors.Wrap(err, "write merged inline helm values") + } + if err := f.Close(); err != nil { + _ = os.Remove(f.Name()) + return func() {}, errors.Wrap(err, "close merged inline helm values") + } + contextLogger.Debug().Msgf("wrote inline helm values to %s", f.Name()) + // Single temp file replaces ValueFiles so inline merge is not applied twice. + valueOpts.ValueFiles = []string{f.Name()} + return func() { + _ = os.Remove(f.Name()) + }, nil +} + +func mergedValuesInlineBytes(opts *RenderOptions) ([]byte, error) { + if opts == nil { + return nil, errors.New("helm RenderOptions is nil") + } + mergeMode := strings.TrimSpace(opts.ValuesMerge) + if mergeMode == "" { + mergeMode = valuesMergeOverride + } + switch mergeMode { + case valuesMergeReplace: + return kyaml.Marshal(opts.ValuesInline) + case valuesMergeMerge, valuesMergeOverride: + default: + return nil, errors.Errorf("invalid helm valuesMerge %q", opts.ValuesMerge) + } + if len(opts.ValuesFiles) == 0 { + return kyaml.Marshal(opts.ValuesInline) + } + baseNode, err := mergeHelmValueFilesToBaseNode(opts.ChartPath, opts.ValuesFiles) + if err != nil { + return nil, err + } + return mergeInlineYAMLWithBase(mergeMode, baseNode, opts.ValuesInline) +} + +// mergeHelmValueFilesToBaseNode merges valueFiles in order (later wins), after constraining paths to the chart tree. +func mergeHelmValueFilesToBaseNode(chartPath string, valueFiles []string) (*kyaml.RNode, error) { + var baseNode *kyaml.RNode + for i, valuesFile := range valueFiles { + safePath, err := valuesFilePathForRead(chartPath, valuesFile) + if err != nil { + return nil, err + } + baseBytes, err := readChartContainedValuesFile(safePath) + if err != nil { + return nil, errors.Wrap(err, "read helm base values file") + } + currentNode, err := kyaml.Parse(string(baseBytes)) + if err != nil { + return nil, errors.Wrap(err, "parse helm base values file") + } + if i == 0 { + baseNode = currentNode + continue + } + // Later values files win over earlier ones (Helm precedence), then inline merges on top. + baseNode, err = merge2.Merge(currentNode, baseNode.Copy(), kyaml.MergeOptions{}) + if err != nil { + return nil, errors.Wrap(err, "merge helm base values files") + } + } + return baseNode, nil +} + +func mergeInlineYAMLWithBase(mergeMode string, baseNode *kyaml.RNode, inline map[string]interface{}) ([]byte, error) { + inlineNode, err := kyaml.FromMap(inline) + if err != nil { + return nil, errors.Wrap(err, "parse inline helm values") + } + var outValues *kyaml.RNode + switch mergeMode { + case valuesMergeOverride: + outValues, err = merge2.Merge(inlineNode, baseNode.Copy(), kyaml.MergeOptions{}) + case valuesMergeMerge: + outValues, err = merge2.Merge(baseNode, inlineNode.Copy(), kyaml.MergeOptions{}) + } + if err != nil { + return nil, errors.Wrap(err, "merge helm inline values") + } + return []byte(outValues.MustString()), nil +} + +// readChartContainedValuesFile reads bytes from absPath; absPath must come from valuesFilePathForRead. +func readChartContainedValuesFile(absPath string) ([]byte, error) { + //nolint:gosec // G304: absPath is only ever valuesFilePathForRead output (under chart directory). + return os.ReadFile(absPath) +} + +// valuesFilePathForRead returns an absolute path to valuesFile only if it resolves under the chart directory. +func valuesFilePathForRead(chartPath, valuesFile string) (string, error) { + chartAbs, err := filepath.Abs(chartPath) + if err != nil { + return "", errors.Wrap(err, "resolve chart path") + } + chartAbs = filepath.Clean(chartAbs) + var p string + if filepath.IsAbs(valuesFile) { + p = filepath.Clean(valuesFile) + } else { + p = filepath.Clean(filepath.Join(chartAbs, valuesFile)) + } + rel, err := filepath.Rel(chartAbs, p) + if err != nil || !filepath.IsLocal(rel) { + return "", errors.Errorf("values file %q is outside chart directory", valuesFile) + } + return p, nil +} + +func isHelmTestTemplate(path string) bool { + path = filepath.ToSlash(path) + return strings.Contains(path, "/tests/") || strings.HasPrefix(path, "tests/") +} + +func splitManifestYAML(template *release.Release) (*[]splitManifest, error) { + sources := make([]*chart.File, 0) + sources = updateName(sources, template.Chart, template.Chart.Name()) + var splitedManifest []splitManifest + splitedSource := strings.Split(template.Manifest, "---") + origData := toMap(sources) + for _, splited := range splitedSource { + var lineID string + for _, line := range strings.Split(splited, "\n") { + if strings.Contains(line, kicsHelmID) { + lineID = line + break + } + } + path := strings.Split(strings.TrimPrefix(splited, "\n# Source: "), "\n") + if path[0] == "" { + continue + } + if origData[filepath.FromSlash(path[0])] == nil { + continue + } + idMap, err := getIDMap(origData[filepath.FromSlash(path[0])]) + if err != nil { + return nil, err + } + splitedManifest = append(splitedManifest, splitManifest{ + path: path[0], + content: []byte(strings.ReplaceAll(splited, "\r", "")), + original: origData[filepath.FromSlash(path[0])], + splitID: lineID, + splitIDMap: idMap, + }) + } + return &splitedManifest, nil +} + +func toMap(files []*chart.File) map[string][]byte { + mapFiles := make(map[string][]byte) + for _, file := range files { + mapFiles[file.Name] = []byte(strings.ReplaceAll(string(file.Data), "\r", "")) + } + return mapFiles +} + +func updateName(template []*chart.File, charts *chart.Chart, name string) []*chart.File { + if name != charts.Name() { + name = filepath.Join(name, charts.Name()) + } + for _, temp := range charts.Templates { + temp.Name = filepath.Join(name, temp.Name) + } + template = append(template, charts.Templates...) + for _, dep := range charts.Dependencies() { + template = updateName(template, dep, filepath.Join(name, "charts")) + } + return template +} + +func getIDMap(originalData []byte) (map[int]interface{}, error) { + ids := make(map[int]interface{}) + mapLines := make(map[int]int) + idHelm := -1 + for line, stringLine := range strings.Split(string(originalData), "\n") { + if strings.Contains(stringLine, kicsHelmID) { + id, err := strconv.Atoi(strings.TrimSuffix(strings.TrimPrefix(stringLine, kicsHelmID), ":")) + if err != nil { + return nil, err + } + if idHelm == -1 { + idHelm = id + mapLines[line] = line + } else { + ids[idHelm] = mapLines + mapLines = make(map[int]int) + idHelm = id + mapLines[line] = line + } + } else if idHelm != -1 { + mapLines[line] = line + } + } + ids[idHelm] = mapLines + + return ids, nil +} diff --git a/pkg/resolver/helm/render_test.go b/pkg/resolver/helm/render_test.go new file mode 100644 index 000000000..291164343 --- /dev/null +++ b/pkg/resolver/helm/render_test.go @@ -0,0 +1,148 @@ +package helm + +import ( + "context" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "helm.sh/helm/v3/pkg/cli/values" +) + +func testHelmFixtureChartDir(t *testing.T) string { + t.Helper() + _, file, _, ok := runtime.Caller(0) + require.True(t, ok) + return filepath.Join(filepath.Dir(file), "..", "..", "..", "test", "fixtures", "test_helm") +} + +func TestRenderChart_invalidChartPath(t *testing.T) { + ctx := context.Background() + _, err := RenderChart(ctx, &RenderOptions{ + ChartPath: "/this/path/does/not/exist/chart", + IncludeCRDs: true, + }) + require.Error(t, err) +} + +func TestRenderChart_valuesInlineAndSkipTests(t *testing.T) { + ctx := context.Background() + chartDir := testHelmFixtureChartDir(t) + out, err := RenderChart(ctx, &RenderOptions{ + ChartPath: chartDir, + IncludeCRDs: true, + SkipTests: true, + ValuesInline: map[string]interface{}{ + "service": map[string]interface{}{ + "port": 9090, + }, + }, + }) + require.NoError(t, err) + require.NotEmpty(t, out.Resources) + var joined strings.Builder + for _, r := range out.Resources { + joined.Write(r.Content) + } + require.Contains(t, joined.String(), "9090") +} + +func TestMergedValuesInlineBytes_UsesAllValuesFilesBeforeInline(t *testing.T) { + dir := t.TempDir() + base := filepath.Join(dir, "base.yaml") + override := filepath.Join(dir, "override.yaml") + require.NoError(t, os.WriteFile(base, []byte("service:\n port: 80\n type: ClusterIP\n"), 0o600)) + require.NoError(t, os.WriteFile(override, []byte("service:\n port: 8080\n"), 0o600)) + + out, err := mergedValuesInlineBytes(&RenderOptions{ + ChartPath: dir, + ValuesFiles: []string{base, override}, + ValuesInline: map[string]interface{}{ + "service": map[string]interface{}{ + "type": "LoadBalancer", + }, + }, + ValuesMerge: valuesMergeOverride, + }) + require.NoError(t, err) + txt := string(out) + require.Contains(t, txt, "port: 8080") + require.Contains(t, txt, "type: LoadBalancer") +} + +func TestApplyValuesInline_ReplacesWholeValuesFileStack(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + base := filepath.Join(dir, "base.yaml") + override := filepath.Join(dir, "override.yaml") + require.NoError(t, os.WriteFile(base, []byte("service:\n port: 80\n"), 0o600)) + require.NoError(t, os.WriteFile(override, []byte("service:\n port: 8080\n"), 0o600)) + + valueOpts := &values.Options{ValueFiles: []string{base, override}} + cleanup, err := applyValuesInline(ctx, valueOpts, &RenderOptions{ + ChartPath: dir, + ValuesFiles: []string{base, override}, + ValuesInline: map[string]interface{}{ + "service": map[string]interface{}{ + "type": "LoadBalancer", + }, + }, + ValuesMerge: valuesMergeOverride, + }) + require.NoError(t, err) + defer cleanup() + require.Len(t, valueOpts.ValueFiles, 1) + require.NotEqual(t, base, valueOpts.ValueFiles[0]) + require.NotEqual(t, override, valueOpts.ValueFiles[0]) +} + +func TestApplyValuesInline_CleanupRemovesTempFile(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + base := filepath.Join(dir, "base.yaml") + require.NoError(t, os.WriteFile(base, []byte("service:\n port: 80\n"), 0o600)) + + valueOpts := &values.Options{ValueFiles: []string{base}} + cleanup, err := applyValuesInline(ctx, valueOpts, &RenderOptions{ + ChartPath: dir, + ValuesFiles: []string{base}, + ValuesInline: map[string]interface{}{ + "service": map[string]interface{}{ + "type": "LoadBalancer", + }, + }, + ValuesMerge: valuesMergeOverride, + }) + require.NoError(t, err) + require.Len(t, valueOpts.ValueFiles, 1) + tempPath := valueOpts.ValueFiles[0] + _, statErr := os.Stat(tempPath) + require.NoError(t, statErr) + + cleanup() + + _, statErr = os.Stat(tempPath) + require.Error(t, statErr) + require.True(t, os.IsNotExist(statErr)) +} + +func TestRenderChart_PreservesExplicitReleaseName(t *testing.T) { + ctx := context.Background() + chartDir := testHelmFixtureChartDir(t) + out, err := RenderChart(ctx, &RenderOptions{ + ChartPath: chartDir, + ReleaseName: "custom-release", + IncludeCRDs: true, + ValuesInline: map[string]interface{}{}, + }) + require.NoError(t, err) + require.NotEmpty(t, out.Resources) + var joined strings.Builder + for _, r := range out.Resources { + joined.Write(r.Content) + } + require.Contains(t, joined.String(), "custom-release") +} diff --git a/pkg/resolver/helm/resolver.go b/pkg/resolver/helm/resolver.go index c8e6ae4f7..b86495195 100644 --- a/pkg/resolver/helm/resolver.go +++ b/pkg/resolver/helm/resolver.go @@ -2,196 +2,101 @@ package helm import ( "context" + "os" "path/filepath" - "regexp" - "strconv" "strings" "github.com/DataDog/datadog-iac-scanner/pkg/logger" "github.com/DataDog/datadog-iac-scanner/pkg/model" masterUtils "github.com/DataDog/datadog-iac-scanner/pkg/utils" - "github.com/pkg/errors" - "helm.sh/helm/v3/pkg/chart" - "helm.sh/helm/v3/pkg/cli/values" - "helm.sh/helm/v3/pkg/release" ) -// Resolver is an instance of the helm resolver +// Resolver is an instance of the helm preprocessor. type Resolver struct { + IncludeCRDs bool } -// splitManifest keeps the information of the manifest splitted by source -type splitManifest struct { - path string - content []byte - original []byte - splitID string - splitIDMap map[int]interface{} +// NewResolver returns a Helm preprocessor with CRDs included in rendered output (recommended default). +func NewResolver() *Resolver { + return &Resolver{IncludeCRDs: true} } -const ( - kicsHelmID = "# KICS_HELM_ID_" -) +// NewResolverWithIncludeCRDs returns a Helm preprocessor with explicit IncludeCRDs behavior. +func NewResolverWithIncludeCRDs(includeCRDs bool) *Resolver { + return &Resolver{IncludeCRDs: includeCRDs} +} + +// Name implements resolver.Preprocessor. +func (r *Resolver) Name() string { + return "helm" +} -// Resolve will render the passed helm chart and return its content ready for parsing +// Detect returns (KindHELM, true) when path is a chart directory. +func (r *Resolver) Detect(path string) (model.FileKind, bool) { + _, err := os.Stat(filepath.Join(path, "Chart.yaml")) + if err != nil { + return model.KindCOMMON, false + } + return model.KindHELM, true +} + +// Resolve renders the passed helm chart and returns manifests ready for parsing. func (r *Resolver) Resolve(ctx context.Context, filePath string) (model.ResolvedFiles, error) { contextLogger := logger.FromContext(ctx) contextLogger.Debug().Msg("Resolving Helm files") - // handle panic during resolve process defer func() { - if r := recover(); r != nil { + if rec := recover(); rec != nil { errMessage := "Recovered from panic during resolve of file " + filePath - masterUtils.HandlePanic(ctx, r, errMessage) + masterUtils.HandlePanic(ctx, rec, errMessage) } }() - splits, excluded, err := renderHelm(ctx, filePath) - if err != nil { // return error to be logged - return model.ResolvedFiles{}, errors.New("failed to render helm chart") + + rc, err := RenderChart(ctx, &RenderOptions{ + ChartPath: filePath, + IncludeCRDs: r.IncludeCRDs, + }) + if err != nil { + return model.ResolvedFiles{ + Diagnostics: []model.ResolverDiagnostic{ + { + FilePath: filePath, + Message: err.Error(), + QueryID: "helm-render-failed", + Line: 1, + }, + }, + }, nil } + var rfiles = model.ResolvedFiles{ - Excluded: excluded, + Excluded: rc.Excluded, } - contextLogger.Debug().Msgf("Processing %d helm manifest splits from chart '%s'", len(*splits), filePath) - for _, split := range *splits { + contextLogger.Debug().Msgf("Processing %d helm manifest splits from chart '%s'", len(rc.Resources), filePath) + for _, res := range rc.Resources { subFolder := filepath.Base(filePath) - - splitPath := strings.Split(split.path, getPathSeparator(split.path)) - + splitPath := strings.Split(res.SourceFile, getPathSeparator(res.SourceFile)) splited := filepath.Join(splitPath[1:]...) - origpath := filepath.Join(filepath.Dir(filePath), subFolder, splited) - rfiles.File = append(rfiles.File, model.ResolvedHelm{ + rfiles.File = append(rfiles.File, model.ResolvedVirtual{ FileName: origpath, - Content: split.content, - OriginalData: split.original, - SplitID: split.splitID, - IDInfo: split.splitIDMap, + Content: res.Content, + OriginalData: res.Original, + SplitID: res.SplitID, + IDInfo: res.IDInfo, }) } - contextLogger.Debug().Msgf("Successfully processed %d helm files from chart '%s'", len(*splits), filePath) + contextLogger.Debug().Msgf("Successfully processed %d helm files from chart '%s'", len(rc.Resources), filePath) return rfiles, nil } -// SupportedTypes returns the supported fileKinds for this resolver +// SupportedTypes returns the supported file kinds for this preprocessor. func (r *Resolver) SupportedTypes() []model.FileKind { return []model.FileKind{model.KindHELM} } -// renderHelm will use helm library to render helm charts -func renderHelm(ctx context.Context, path string) (*[]splitManifest, []string, error) { - contextLogger := logger.FromContext(ctx) - client := newClient(ctx) - contextLogger.Debug().Msg("Running helm install") - manifest, excluded, err := runInstall(ctx, []string{path}, client, &values.Options{}) - if err != nil { - contextLogger.Error().Msgf("failed to run helm install '%s': %s", path, err) - return nil, []string{}, err - } - splitted, err := splitManifestYAML(manifest) - if err != nil { - contextLogger.Error().Msgf("failed to split helm manifest YAML for chart '%s': %s", path, err) - return nil, []string{}, err - } - return splitted, excluded, nil -} - -// splitManifestYAML will split the rendered file and return its content by template as well as the template path -func splitManifestYAML(template *release.Release) (*[]splitManifest, error) { - sources := make([]*chart.File, 0) - sources = updateName(sources, template.Chart, template.Chart.Name()) - var splitedManifest []splitManifest - splitedSource := strings.Split(template.Manifest, "---") // split manifest by '---' - origData := toMap(sources) - for _, splited := range splitedSource { - var lineID string - for _, line := range strings.Split(splited, "\n") { - if strings.Contains(line, kicsHelmID) { - lineID = line // get auxiliary line id - break - } - } - path := strings.Split(strings.TrimPrefix(splited, "\n# Source: "), "\n") // get source of split yaml - // ignore auxiliary files used to render chart - if path[0] == "" { - continue - } - if origData[filepath.FromSlash(path[0])] == nil { - continue - } - idMap, err := getIDMap(origData[filepath.FromSlash(path[0])]) - if err != nil { - return nil, err - } - splitedManifest = append(splitedManifest, splitManifest{ - path: path[0], - content: []byte(strings.ReplaceAll(splited, "\r", "")), - original: origData[filepath.FromSlash(path[0])], // get original data from template - splitID: lineID, - splitIDMap: idMap, - }) - } - return &splitedManifest, nil -} - -// toMap will convert to map original data having the path as it's key -func toMap(files []*chart.File) map[string][]byte { - mapFiles := make(map[string][]byte) - for _, file := range files { - mapFiles[file.Name] = []byte(strings.ReplaceAll(string(file.Data), "\r", "")) - } - return mapFiles -} - -// updateName will update the templates name as well as its dependencies -func updateName(template []*chart.File, charts *chart.Chart, name string) []*chart.File { - if name != charts.Name() { - name = filepath.Join(name, charts.Name()) - } - for _, temp := range charts.Templates { - temp.Name = filepath.Join(name, temp.Name) - } - template = append(template, charts.Templates...) - for _, dep := range charts.Dependencies() { - template = updateName(template, dep, filepath.Join(name, "charts")) - } - return template -} - -// getIdMap will construct a map with ids with the corresponding lines as keys -// for use in detector -func getIDMap(originalData []byte) (map[int]interface{}, error) { - ids := make(map[int]interface{}) - mapLines := make(map[int]int) - idHelm := -1 - for line, stringLine := range strings.Split(string(originalData), "\n") { - if strings.Contains(stringLine, kicsHelmID) { - id, err := strconv.Atoi(strings.TrimSuffix(strings.TrimPrefix(stringLine, kicsHelmID), ":")) - if err != nil { - return nil, err - } - if idHelm == -1 { - idHelm = id - mapLines[line] = line - } else { - ids[idHelm] = mapLines - mapLines = make(map[int]int) - idHelm = id - mapLines[line] = line - } - } else if idHelm != -1 { - mapLines[line] = line - } - } - ids[idHelm] = mapLines - - return ids, nil -} - func getPathSeparator(path string) string { - if matched, err := regexp.MatchString(`[a-zA-Z0-9_\/-]+(\[a-zA-Z0-9_\/-]+)*`, path); matched && err == nil { - return "/" - } else if matched, err := regexp.MatchString(`[a-z0-9_.$-]+(\\[a-z0-9_.$-]+)*`, path); matched && err == nil { + if strings.Contains(path, "\\") { return "\\" } - return "" + return "/" } diff --git a/pkg/resolver/helm/resolver_test.go b/pkg/resolver/helm/resolver_test.go index 7ec1d842d..9ca227149 100644 --- a/pkg/resolver/helm/resolver_test.go +++ b/pkg/resolver/helm/resolver_test.go @@ -22,7 +22,8 @@ func TestHelm_SupportedTypes(t *testing.T) { } func TestHelm_Resolve(t *testing.T) { //nolint - res := &Resolver{} + // Match legacy snapshots: CRDs were off before the refactor default flip. + res := NewResolverWithIncludeCRDs(false) type args struct { filePath string } @@ -38,7 +39,7 @@ func TestHelm_Resolve(t *testing.T) { //nolint filePath: filepath.FromSlash("../../../test/fixtures/test_helm"), }, want: model.ResolvedFiles{ - File: []model.ResolvedHelm{ + File: []model.ResolvedVirtual{ { SplitID: "# KICS_HELM_ID_0:", FileName: filepath.FromSlash("../../../test/fixtures/test_helm/templates/service.yaml"), @@ -95,8 +96,14 @@ spec: args: args{ filePath: filepath.FromSlash("../../../test/fixtures/all_auth_users_get_read_access"), }, - want: model.ResolvedFiles{}, - wantErr: true, + want: model.ResolvedFiles{ + Diagnostics: []model.ResolverDiagnostic{{ + FilePath: filepath.FromSlash("../../../test/fixtures/all_auth_users_get_read_access"), + QueryID: "helm-render-failed", + Line: 1, + }}, + }, + wantErr: false, }, { name: "test_with_dependencies", @@ -104,7 +111,7 @@ spec: filePath: filepath.FromSlash("../../../test/fixtures/test_helm_subchart"), }, want: model.ResolvedFiles{ - File: []model.ResolvedHelm{ + File: []model.ResolvedVirtual{ { FileName: filepath.FromSlash("../../../test/fixtures/test_helm_subchart/templates/serviceaccount.yaml"), SplitID: "# KICS_HELM_ID_1:", @@ -199,10 +206,15 @@ spec: if (err != nil) != tt.wantErr { t.Errorf("Resolve() = %v, wantErr = %v", err, tt.wantErr) } + if tt.name == "err_resolve" { + require.Len(t, got.Diagnostics, 1) + require.Equal(t, tt.want.Diagnostics[0].QueryID, got.Diagnostics[0].QueryID) + return + } if !reflect.DeepEqual(got.File, tt.want.File) { t.Errorf("Resolve() = %v, want = %v", got, tt.want) } - if err == nil { + if err == nil && len(got.Diagnostics) == 0 { require.NotEmpty(t, got.Excluded) } }) diff --git a/pkg/resolver/resolver.go b/pkg/resolver/resolver.go index a9305e0be..a5748b96e 100644 --- a/pkg/resolver/resolver.go +++ b/pkg/resolver/resolver.go @@ -7,81 +7,81 @@ package resolver import ( "context" - "os" - "path/filepath" "github.com/DataDog/datadog-iac-scanner/pkg/logger" "github.com/DataDog/datadog-iac-scanner/pkg/model" ) -// kindResolver is a type of resolver interface (ex: helm resolver) -// Resolve will render file/template -// SupportedTypes will return the file kinds that the resolver supports -type kindResolver interface { - Resolve(ctx context.Context, filePath string) (model.ResolvedFiles, error) +// Preprocessor expands composed IaC (Helm, Kustomize, …) into files for parsing. +type Preprocessor interface { + Resolve(ctx context.Context, rootDir string) (model.ResolvedFiles, error) SupportedTypes() []model.FileKind + Name() string + Detect(path string) (model.FileKind, bool) } -// Resolver is a struct containing the resolvers by file kind +// Resolver dispatches to preprocessors by file kind. type Resolver struct { - resolvers map[model.FileKind]kindResolver + byKind map[model.FileKind]Preprocessor + ordered []Preprocessor } -// Builder is a struct used to create a new resolver +// Builder constructs a Resolver from preprocessors. type Builder struct { - resolvers []kindResolver + preprocessors []Preprocessor } -// NewBuilder creates a new Builder's reference +// NewBuilder creates a new Builder. func NewBuilder() *Builder { return &Builder{} } -// Add will add kindResolvers for building the resolver -func (b *Builder) Add(ctx context.Context, p kindResolver) *Builder { +// Add registers a preprocessor (call order is preserved for GetType detection). +func (b *Builder) Add(ctx context.Context, p Preprocessor) *Builder { contextLogger := logger.FromContext(ctx) - contextLogger.Debug().Msgf("resolver.Add()") - b.resolvers = append(b.resolvers, p) + contextLogger.Debug().Msgf("resolver.Add(%s)", p.Name()) + b.preprocessors = append(b.preprocessors, p) return b } -// Build will create a new instance of a resolver +// Build creates the Resolver. Later preprocessors overwrite byKind for the same FileKind. func (b *Builder) Build(ctx context.Context) (*Resolver, error) { contextLogger := logger.FromContext(ctx) contextLogger.Debug().Msg("resolver.Build()") - resolvers := make(map[model.FileKind]kindResolver, len(b.resolvers)) - for _, resolver := range b.resolvers { - for _, typeRes := range resolver.SupportedTypes() { - resolvers[typeRes] = resolver + byKind := make(map[model.FileKind]Preprocessor) + for _, p := range b.preprocessors { + for _, t := range p.SupportedTypes() { + byKind[t] = p } } return &Resolver{ - resolvers: resolvers, + byKind: byKind, + ordered: append([]Preprocessor(nil), b.preprocessors...), }, nil } -// Resolve will resolve the files according to its type +// Resolve runs the preprocessor for the given kind. func (r *Resolver) Resolve(ctx context.Context, filePath string, kind model.FileKind) (model.ResolvedFiles, error) { contextLogger := logger.FromContext(ctx) - if r, ok := r.resolvers[kind]; ok { - obj, err := r.Resolve(ctx, filePath) + if p, ok := r.byKind[kind]; ok { + obj, err := p.Resolve(ctx, filePath) if err != nil { return model.ResolvedFiles{}, err } contextLogger.Debug().Msgf("resolver.Resolve() rendered file: %s", filePath) return obj, nil } - // need to log here return model.ResolvedFiles{}, nil } -// GetType will analyze the filepath to determine which resolver to use +// GetType picks a preprocessor for a directory; registration order matters (Helm before Kustomize for Chart.yaml dirs). func (r *Resolver) GetType(filePath string) model.FileKind { - _, err := os.Stat(filepath.Join(filePath, "Chart.yaml")) - if err == nil { - return model.KindHELM + for _, p := range r.ordered { + if k, ok := p.Detect(filePath); ok { + return k + } } return model.KindCOMMON } From ec45171296e35bc39dcea9296de08408e48f846d Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Fri, 24 Apr 2026 13:14:11 +0200 Subject: [PATCH 03/17] Add sandbox temp dirs and a kyaml BoundedFS that rejects paths outside the repo root. --- pkg/resolver/sandbox/bounded_fs.go | 235 ++++++++++++++++++++++++ pkg/resolver/sandbox/bounded_fs_test.go | 56 ++++++ pkg/resolver/sandbox/sandbox.go | 31 ++++ 3 files changed, 322 insertions(+) create mode 100644 pkg/resolver/sandbox/bounded_fs.go create mode 100644 pkg/resolver/sandbox/bounded_fs_test.go create mode 100644 pkg/resolver/sandbox/sandbox.go diff --git a/pkg/resolver/sandbox/bounded_fs.go b/pkg/resolver/sandbox/bounded_fs.go new file mode 100644 index 000000000..cfabcbb11 --- /dev/null +++ b/pkg/resolver/sandbox/bounded_fs.go @@ -0,0 +1,235 @@ +package sandbox + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "sigs.k8s.io/kustomize/kyaml/filesys" +) + +// BoundedFS wraps a kustomize FileSystem and rejects paths outside repoRoot. +type BoundedFS struct { + inner filesys.FileSystem + repoAbs string +} + +// NewBoundedFS returns a disk-backed filesystem bounded to repoRoot (must exist). +func NewBoundedFS(repoRoot string) (*BoundedFS, error) { + abs, err := canonicalPath(repoRoot) + if err != nil { + return nil, err + } + if st, err := os.Stat(abs); err != nil || !st.IsDir() { + return nil, fmt.Errorf("sandbox: repo root %q is not a directory", abs) + } + return &BoundedFS{ + inner: filesys.MakeFsOnDisk(), + repoAbs: filepath.Clean(abs), + }, nil +} + +func (b *BoundedFS) under(abs string) bool { + cp, err := canonicalPath(abs) + if err != nil { + cp = filepath.Clean(abs) + } + return cp == b.repoAbs || strings.HasPrefix(cp, b.repoAbs+string(filepath.Separator)) +} + +func (b *BoundedFS) checkCleaned(d filesys.ConfirmedDir, f string, resolveLeaf bool) error { + full, err := canonicalPath(filepath.Join(string(d), f)) + if err != nil { + full = filepath.Clean(filepath.Join(string(d), f)) + } + if err := b.checkParents(full); err != nil { + return err + } + if !resolveLeaf { + if parent := filepath.Dir(full); parent != full { + if ev, err := filepath.EvalSymlinks(parent); err == nil { + full = filepath.Join(filepath.Clean(ev), filepath.Base(full)) + } + } + } + if !b.under(full) { + return fmt.Errorf("path outside scan root: %s", full) + } + return nil +} + +func (b *BoundedFS) checkParents(path string) error { + cur := filepath.Clean(path) + if cur == b.repoAbs { + return nil + } + for { + parent := filepath.Dir(cur) + if parent == cur { + return nil + } + ev, err := canonicalPath(parent) + if err == nil { + if ev == b.repoAbs { + return nil + } + if !strings.HasPrefix(ev, b.repoAbs+string(filepath.Separator)) { + return fmt.Errorf("path outside scan root: %s", path) + } + } + cur = parent + } +} + +func canonicalPath(path string) (string, error) { + abs, err := filepath.Abs(path) + if err != nil { + return "", err + } + if ev, err := filepath.EvalSymlinks(abs); err == nil { + return filepath.Clean(ev), nil + } + return filepath.Clean(abs), nil +} + +func (b *BoundedFS) cleanedPath(path string, resolveLeaf bool) (filesys.ConfirmedDir, string, error) { + d, f, err := b.inner.CleanedAbs(path) + if err != nil { + return "", "", err + } + if err := b.checkCleaned(d, f, resolveLeaf); err != nil { + return "", "", err + } + return d, f, nil +} + +// Create implements filesys.FileSystem. +func (b *BoundedFS) Create(path string) (filesys.File, error) { + if _, _, err := b.cleanedPath(path, false); err != nil { + return nil, err + } + return b.inner.Create(path) +} + +// Mkdir implements filesys.FileSystem. +func (b *BoundedFS) Mkdir(path string) error { + if _, _, err := b.cleanedPath(path, false); err != nil { + return err + } + return b.inner.Mkdir(path) +} + +// MkdirAll implements filesys.FileSystem. +func (b *BoundedFS) MkdirAll(path string) error { + if _, _, err := b.cleanedPath(path, false); err != nil { + return err + } + return b.inner.MkdirAll(path) +} + +// RemoveAll implements filesys.FileSystem. +func (b *BoundedFS) RemoveAll(path string) error { + if _, _, err := b.cleanedPath(path, true); err != nil { + return err + } + return b.inner.RemoveAll(path) +} + +// Open implements filesys.FileSystem. +func (b *BoundedFS) Open(path string) (filesys.File, error) { + if _, _, err := b.cleanedPath(path, true); err != nil { + return nil, err + } + return b.inner.Open(path) +} + +// IsDir implements filesys.FileSystem. +func (b *BoundedFS) IsDir(path string) bool { + d, f, err := b.cleanedPath(path, true) + if err != nil { + return false + } + _ = d + _ = f + return b.inner.IsDir(path) +} + +// ReadDir implements filesys.FileSystem. +func (b *BoundedFS) ReadDir(path string) ([]string, error) { + if _, _, err := b.cleanedPath(path, true); err != nil { + return nil, err + } + return b.inner.ReadDir(path) +} + +// CleanedAbs implements filesys.FileSystem. +func (b *BoundedFS) CleanedAbs(path string) (filesys.ConfirmedDir, string, error) { + return b.cleanedPath(path, true) +} + +// Exists implements filesys.FileSystem. +func (b *BoundedFS) Exists(path string) bool { + d, f, err := b.cleanedPath(path, true) + if err != nil { + return false + } + _ = d + _ = f + return b.inner.Exists(path) +} + +// Glob implements filesys.FileSystem. +func (b *BoundedFS) Glob(pattern string) ([]string, error) { + matches, err := b.inner.Glob(pattern) + if err != nil { + return nil, err + } + var out []string + for _, m := range matches { + d, f, errC := b.cleanedPath(m, true) + if errC != nil { + continue + } + _ = d + _ = f + out = append(out, m) + } + return out, nil +} + +// ReadFile implements filesys.FileSystem. +func (b *BoundedFS) ReadFile(path string) ([]byte, error) { + if _, _, err := b.cleanedPath(path, true); err != nil { + return nil, err + } + return b.inner.ReadFile(path) +} + +// WriteFile implements filesys.FileSystem. +func (b *BoundedFS) WriteFile(path string, data []byte) error { + if _, _, err := b.cleanedPath(path, false); err != nil { + return err + } + return b.inner.WriteFile(path, data) +} + +// Walk implements filesys.FileSystem. +func (b *BoundedFS) Walk(path string, walkFn filepath.WalkFunc) error { + if _, _, err := b.cleanedPath(path, true); err != nil { + return err + } + return b.inner.Walk(path, func(p string, info os.FileInfo, err error) error { + if err != nil { + return walkFn(p, info, err) + } + cd, cf, errC := b.inner.CleanedAbs(p) + if errC != nil { + return walkFn(p, info, errC) + } + if errB := b.checkCleaned(cd, cf, true); errB != nil { + return filepath.SkipDir + } + return walkFn(p, info, nil) + }) +} diff --git a/pkg/resolver/sandbox/bounded_fs_test.go b/pkg/resolver/sandbox/bounded_fs_test.go new file mode 100644 index 000000000..8b0158e94 --- /dev/null +++ b/pkg/resolver/sandbox/bounded_fs_test.go @@ -0,0 +1,56 @@ +package sandbox + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestBoundedFS_RejectsSymlinkEscape(t *testing.T) { + root := t.TempDir() + outside := filepath.Join(t.TempDir(), "secret.yaml") + require.NoError(t, os.WriteFile(outside, []byte("secret"), 0o600)) + link := filepath.Join(root, "escape.yaml") + require.NoError(t, os.Symlink(outside, link)) + + fs, err := NewBoundedFS(root) + require.NoError(t, err) + _, err = fs.ReadFile(link) + require.Error(t, err, "read through symlink to outside root should be rejected") +} + +func TestBoundedFS_RejectsTraversal(t *testing.T) { + root := t.TempDir() + sub := filepath.Join(root, "safe") + require.NoError(t, os.MkdirAll(sub, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(sub, "a.yaml"), []byte("k: v\n"), 0o600)) + + fs, err := NewBoundedFS(root) + require.NoError(t, err) + _, err = fs.ReadFile(filepath.Join(sub, "a.yaml")) + require.NoError(t, err) + + outside := filepath.Join(root, "..", "outside") + _, err = fs.ReadFile(outside) + require.Error(t, err) +} + +func TestBoundedFS_RejectsWriteThroughSymlinkedParent(t *testing.T) { + root := t.TempDir() + outsideRoot := t.TempDir() + linkDir := filepath.Join(root, "linked") + require.NoError(t, os.Symlink(outsideRoot, linkDir)) + + fs, err := NewBoundedFS(root) + require.NoError(t, err) + + target := filepath.Join(linkDir, "escape.yaml") + err = fs.WriteFile(target, []byte("secret")) + require.Error(t, err, "write through symlinked parent should be rejected") + + _, statErr := os.Stat(filepath.Join(outsideRoot, "escape.yaml")) + require.Error(t, statErr) + require.True(t, os.IsNotExist(statErr)) +} diff --git a/pkg/resolver/sandbox/sandbox.go b/pkg/resolver/sandbox/sandbox.go new file mode 100644 index 000000000..5736c8b08 --- /dev/null +++ b/pkg/resolver/sandbox/sandbox.go @@ -0,0 +1,31 @@ +// Package sandbox: per-scan temp dirs for preprocessors. +package sandbox + +import ( + "os" +) + +// Sandbox is a per-scan writable temp directory (0700 from MkdirTemp on Unix), removed on Close. +type Sandbox struct { + RepoRoot string + ScratchDir string +} + +// New creates a sandbox under the OS temp directory (MkdirTemp uses 0700 on Unix). +func New() (*Sandbox, error) { + t, err := os.MkdirTemp("", "datadog-iac-scanner-") + if err != nil { + return nil, err + } + return &Sandbox{ScratchDir: t}, nil +} + +// Close removes the scratch directory (idempotent). +func (s *Sandbox) Close() error { + if s == nil || s.ScratchDir == "" { + return nil + } + err := os.RemoveAll(s.ScratchDir) + s.ScratchDir = "" + return err +} From 1d2c470ee307c6faa64cb98ff8a46841b7a31239 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Fri, 24 Apr 2026 13:14:17 +0200 Subject: [PATCH 04/17] Wire resolver diagnostics through the scanning pipeline, improve virtual-file sink parsing, and treat Kustomize like YAML for JSON filters. --- pkg/kics/resolver_diagnostic_titles.go | 24 +++++ pkg/kics/resolver_diagnostics.go | 133 +++++++++++++++++++++++++ pkg/kics/resolver_sink.go | 21 ++-- pkg/kics/service.go | 25 +++-- pkg/scanner/scanner_test.go | 4 +- 5 files changed, 190 insertions(+), 17 deletions(-) create mode 100644 pkg/kics/resolver_diagnostic_titles.go create mode 100644 pkg/kics/resolver_diagnostics.go diff --git a/pkg/kics/resolver_diagnostic_titles.go b/pkg/kics/resolver_diagnostic_titles.go new file mode 100644 index 000000000..38da99eef --- /dev/null +++ b/pkg/kics/resolver_diagnostic_titles.go @@ -0,0 +1,24 @@ +package kics + +// Stable human-readable titles for resolver diagnostics (no Rego metadata). +var resolverDiagnosticTitles = map[string]string{ + "kustomize-render-failed": "Kustomize render failed", + "kustomize-remote-disallowed": "Kustomize remote reference blocked", + "kustomize-max-fetch-exceeded": "Kustomize scratch size limit exceeded", + "kustomize-exec-plugin-disabled": "Kustomize exec-style plugins disabled", + "kustomize-helm-inflation-disabled": "Kustomize Helm inflation disabled", + "kustomize-helm-prepass-failed": "Kustomize Helm prepass failed", + "kustomize-helm-chart-missing": "Kustomize Helm chart missing", + "kustomize-helm-remote-chart-invalid": "Kustomize Helm remote chart invalid", + "kustomize-helm-values-invalid": "Kustomize Helm values invalid", + "kustomize-helm-render-failed": "Kustomize Helm chart render failed", + "kustomize-helm-write-failed": "Kustomize Helm rendered manifest write failed", + "kustomize-transformer-path-missing": "Kustomize transformer patch path missing", +} + +func resolverDiagnosticQueryName(queryID string) string { + if t, ok := resolverDiagnosticTitles[queryID]; ok { + return t + } + return "IaC resolver notice: " + queryID +} diff --git a/pkg/kics/resolver_diagnostics.go b/pkg/kics/resolver_diagnostics.go new file mode 100644 index 000000000..d9da828db --- /dev/null +++ b/pkg/kics/resolver_diagnostics.go @@ -0,0 +1,133 @@ +package kics + +import ( + "context" + "os" + "path/filepath" + "sync" + + "github.com/DataDog/datadog-iac-scanner/pkg/model" +) + +type ResolverDiagnosticsState struct { + mu sync.Mutex + seen map[string]struct{} +} + +func NewResolverDiagnosticsState() *ResolverDiagnosticsState { + return &ResolverDiagnosticsState{ + seen: make(map[string]struct{}), + } +} + +func (s *ResolverDiagnosticsState) FirstSeen(scanID string, d model.ResolverDiagnostic) bool { + if s == nil { + return true + } + key := resolverDiagnosticDedupKey(scanID, d) + s.mu.Lock() + defer s.mu.Unlock() + if _, ok := s.seen[key]; ok { + return false + } + s.seen[key] = struct{}{} + return true +} + +func (s *Service) saveResolverDiagnostics(ctx context.Context, scanID string, diags []model.ResolverDiagnostic) error { + if len(diags) == 0 { + return nil + } + var vulns []model.Vulnerability + for _, d := range diags { + if s.ResolverDiagnostics != nil && !s.ResolverDiagnostics.FirstSeen(scanID, d) { + continue + } + line := d.Line + if line < 1 { + line = 1 + } + vulns = append(vulns, model.Vulnerability{ + ScanID: scanID, + QueryID: d.QueryID, + QueryName: resolverDiagnosticQueryName(d.QueryID), + FileName: d.FilePath, + Line: line, + VulnerabilityLocation: model.ResourceLocation{ + Start: model.ResourceLine{Line: line, Col: 1}, + End: model.ResourceLine{Line: line, Col: 1}, + }, + Severity: model.SeverityInfo, + Category: "Resolver", + Description: d.Message, + Platform: resolverDiagnosticPlatform(s, d), + IssueType: model.IssueTypeIncorrectValue, + SearchKey: "resolver_diagnostic." + d.QueryID, + SearchLine: line, + SearchValue: d.Message, + }) + } + if len(vulns) == 0 { + return nil + } + return s.Storage.SaveVulnerabilities(ctx, vulns) +} + +func resolverDiagnosticPlatform(s *Service, d model.ResolverDiagnostic) string { + if d.QueryID == "" { + if s == nil || s.Parser == nil || len(s.Parser.Platform) == 0 { + return "" + } + return s.Parser.Platform[0] + } + if s != nil && s.Resolver != nil { + targetPath := d.FilePath + if targetPath != "" { + if info, err := os.Stat(targetPath); err == nil && !info.IsDir() { + targetPath = filepath.Dir(targetPath) + } + } + switch s.Resolver.GetType(targetPath) { + case model.KindKUSTOMIZE, model.KindHELM: + return "Kubernetes" + } + } + if s == nil || s.Parser == nil || len(s.Parser.Platform) == 0 { + return "" + } + return s.Parser.Platform[0] +} + +func resolverDiagnosticDedupKey(scanID string, d model.ResolverDiagnostic) string { + return joinNull(scanID, d.QueryID, d.FilePath, d.Message, itoaOrZero(d.Line)) +} + +func joinNull(parts ...string) string { + if len(parts) == 0 { + return "" + } + out := parts[0] + for i := 1; i < len(parts); i++ { + out += "\x00" + parts[i] + } + return out +} + +func itoaOrZero(v int) string { + if v == 0 { + return "0" + } + sign := "" + if v < 0 { + sign = "-" + v = -v + } + var digits [20]byte + i := len(digits) + for v > 0 { + i-- + digits[i] = byte('0' + v%10) + v /= 10 + } + return sign + string(digits[i:]) +} diff --git a/pkg/kics/resolver_sink.go b/pkg/kics/resolver_sink.go index c88537710..1472a072b 100644 --- a/pkg/kics/resolver_sink.go +++ b/pkg/kics/resolver_sink.go @@ -34,23 +34,31 @@ func (s *Service) resolverSink( contextLogger.Err(err).Msgf("failed to render file content '%s' with fileType '%s'", filename, kind) return []string{}, err } + if err := s.saveResolverDiagnostics(ctx, scanID, resFiles.Diagnostics); err != nil { + contextLogger.Err(err).Msg("failed to save resolver diagnostics") + } for _, rfile := range resFiles.File { s.Tracker.TrackFileFound(rfile.FileName) + metadataPath := rfile.MetadataPath + if metadataPath == "" { + metadataPath = rfile.FileName + } isMinified := minified.IsMinified(rfile.FileName, rfile.Content) documents, err := s.Parser.Parse(ctx, rfile.FileName, rfile.Content, openAPIResolveReferences, isMinified, maxResolverDepth) if err != nil { - if documents.Kind == "break" { - return []string{}, nil + // Skip this doc but keep iterating so resFiles.Excluded still propagates; + // otherwise the walker re-scans patches / generator inputs as raw YAML. + if documents.Kind != "break" { + contextLogger.Error().Msgf("failed to parse file content '%s' with fileType '%s'", rfile.FileName, kind) } - contextLogger.Error().Msgf("failed to parse file content '%s' with fileType '%s'", rfile.FileName, kind) - return []string{}, nil + continue } if kind == model.KindHELM { ignoreList, errorIL := s.getOriginalIgnoreLines(ctx, - rfile.FileName, rfile.OriginalData, + metadataPath, rfile.OriginalData, openAPIResolveReferences, isMinified, maxResolverDepth) if errorIL == nil { documents.IgnoreLines = ignoreList @@ -62,7 +70,7 @@ func (s *Service) resolverSink( documents.CountLines = bytes.Count(rfile.OriginalData, []byte{'\n'}) + 1 } - fileCommands := s.Parser.CommentsCommands(ctx, rfile.FileName, rfile.OriginalData) + fileCommands := s.Parser.CommentsCommands(ctx, metadataPath, rfile.OriginalData) for _, document := range documents.Docs { _, err = json.Marshal(document) @@ -89,6 +97,7 @@ func (s *Service) resolverSink( ResolvedFiles: documents.ResolvedFiles, LinesOriginalData: utils.SplitLines(string(rfile.OriginalData)), IsMinified: documents.IsMinified, + KustomizeOrigin: rfile.Origin, } s.saveToFile(ctx, &file) } diff --git a/pkg/kics/service.go b/pkg/kics/service.go index 4263fb8c9..03e713ab4 100644 --- a/pkg/kics/service.go +++ b/pkg/kics/service.go @@ -56,15 +56,16 @@ type Tracker interface { // a parser to parse and provide files in format that KICS understand, a inspector that runs the scanning and a tracker to // update scanning numbers type Service struct { - SourceProvider provider.SourceProvider - Storage Storage - Parser *parser.Parser - Inspector *engine.Inspector - Tracker Tracker - Resolver *resolver.Resolver - files model.FileMetadatas - filesMu sync.Mutex - MaxFileSize int + SourceProvider provider.SourceProvider + Storage Storage + Parser *parser.Parser + Inspector *engine.Inspector + Tracker Tracker + Resolver *resolver.Resolver + ResolverDiagnostics *ResolverDiagnosticsState + files model.FileMetadatas + filesMu sync.Mutex + MaxFileSize int } // PrepareSources will prepare the sources to be scanned @@ -248,7 +249,11 @@ func prepareScanDocumentValue(bodyType map[string]interface{}, kind model.FileKi prepareScanDocumentRoot(indx, kind) } case string: - if field, ok := lines[kind]; ok && utils.Contains(key, field) { + lookupKind := kind + if kind == model.KindKUSTOMIZE { + lookupKind = model.KindYAML + } + if field, ok := lines[lookupKind]; ok && utils.Contains(key, field) { bodyType[key] = resolveJSONFilter(value) } } diff --git a/pkg/scanner/scanner_test.go b/pkg/scanner/scanner_test.go index 1b5b89523..b8e319081 100644 --- a/pkg/scanner/scanner_test.go +++ b/pkg/scanner/scanner_test.go @@ -129,6 +129,7 @@ func createServices(types, cloudProviders []string) (serviceSlice, *storage.Memo } store := storage.NewMemoryStorage() + resolverDiagnostics := kics.NewResolverDiagnosticsState() services := make([]*kics.Service, 0, len(combinedParser)) @@ -140,6 +141,7 @@ func createServices(types, cloudProviders []string) (serviceSlice, *storage.Memo Inspector: inspector, Tracker: t, Resolver: combinedResolver, + ResolverDiagnostics: resolverDiagnostics, MaxFileSize: 100, }) } @@ -154,7 +156,7 @@ func createContext(ctx context.Context, timeout time.Duration) testContext { } } -// TODO: fix those tests see K9VULN-8520 +// TODO: re-enable concurrent scan tests once shared scan state is race-free. //func TestScanner_ConcurrentScans(t *testing.T) { // ctx := context.Background() // From b34bdedf7ca9b5a48fa379fe91abb34185895dd0 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Fri, 24 Apr 2026 13:14:21 +0200 Subject: [PATCH 05/17] Introduce the k9-iac-enable-kustomize-resolver flag defaulting to off in the local evaluator. --- pkg/featureflags/evaluator.go | 1 + pkg/featureflags/local_evaluator.go | 1 + 2 files changed, 2 insertions(+) diff --git a/pkg/featureflags/evaluator.go b/pkg/featureflags/evaluator.go index d74385e91..dc7b19ba2 100644 --- a/pkg/featureflags/evaluator.go +++ b/pkg/featureflags/evaluator.go @@ -6,6 +6,7 @@ const ( IacDisableKicsRule = "k9-iac-disable-kics-rule" IacEnableKicsPlatform = "k9-iac-enable-kics-platform" IacEnableKicsHelmResolver = "k9-iac-enable-kics-helm-resolver" + IacEnableKustomizeResolver = "k9-iac-enable-kustomize-resolver" IaCEnableKicsParallelFileParsing = "k9-iac-enable-kics-parallel-file-parsing" ) diff --git a/pkg/featureflags/local_evaluator.go b/pkg/featureflags/local_evaluator.go index 235a93bfe..392ee25bf 100644 --- a/pkg/featureflags/local_evaluator.go +++ b/pkg/featureflags/local_evaluator.go @@ -11,6 +11,7 @@ func NewLocalEvaluatorWithOverrides(overrides map[string]bool) *LocalEvaluator { IacDisableKicsRule: false, IacEnableKicsPlatform: true, IacEnableKicsHelmResolver: true, + IacEnableKustomizeResolver: false, IaCEnableKicsParallelFileParsing: false, } From 41d7616c69e42a6897fd4061b7fed042dcf3390d Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Fri, 24 Apr 2026 13:14:25 +0200 Subject: [PATCH 06/17] Add DefaultYAMLDetectLine as a thin wrapper around the shared path-based YAML detector. --- pkg/detector/default_detect.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/detector/default_detect.go b/pkg/detector/default_detect.go index f70b67d16..e54062b4e 100644 --- a/pkg/detector/default_detect.go +++ b/pkg/detector/default_detect.go @@ -100,3 +100,16 @@ func (d defaultDetectLine) prepareResolvedFiles(resFiles map[string]model.Resolv } return resolvedFiles } + +// DefaultYAMLDetectLine is the shared YAML/JSON line detector for subpackages (e.g. kustomize). +type DefaultYAMLDetectLine struct{} + +// DetectLine forwards to the default path-based YAML detector. +func (DefaultYAMLDetectLine) DetectLine( + ctx context.Context, + file *model.FileMetadata, + searchKey string, + outputLines int, +) model.VulnerabilityLines { + return defaultDetectLine{}.DetectLine(ctx, file, searchKey, outputLines) +} From ec13e91fd97e281e7fdd94176db9808187456a2c Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Fri, 24 Apr 2026 13:14:28 +0200 Subject: [PATCH 07/17] Split the long checkIncluded condition in datadog rule selection to satisfy the line-length linter. --- pkg/engine/source/datadog.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/engine/source/datadog.go b/pkg/engine/source/datadog.go index 6283ebbb3..60a77aee3 100644 --- a/pkg/engine/source/datadog.go +++ b/pkg/engine/source/datadog.go @@ -240,7 +240,9 @@ func checkExcluded(rule *Rule, selection *QueryInspectorParameters) bool { } func checkIncluded(rule *Rule, selection *QueryInspectorParameters) bool { - return (isInCaseInsensitiveNotEmptyList(rule.LegacyId, selection.IncludeQueries.ByIDs) || isInCaseInsensitiveNotEmptyList(&rule.ID, selection.IncludeQueries.ByIDs)) && + byID := isInCaseInsensitiveNotEmptyList(rule.LegacyId, selection.IncludeQueries.ByIDs) || + isInCaseInsensitiveNotEmptyList(&rule.ID, selection.IncludeQueries.ByIDs) + return byID && isInCaseInsensitiveNotEmptyList(&rule.Category, selection.IncludeQueries.ByCategories) && isInCaseInsensitiveNotEmptyList(&rule.Severity, selection.IncludeQueries.BySeverities) } From b126212eac49b38a013e587c88c002c4ef4409e1 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Mon, 27 Apr 2026 10:02:05 +0200 Subject: [PATCH 08/17] Codex comments --- pkg/resolver/helm/render.go | 14 +++++++-- pkg/resolver/helm/render_test.go | 25 ++++++++++++++++ pkg/resolver/sandbox/bounded_fs.go | 6 ++++ pkg/resolver/sandbox/bounded_fs_test.go | 39 +++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 2 deletions(-) diff --git a/pkg/resolver/helm/render.go b/pkg/resolver/helm/render.go index 2b72317d9..d500b67d1 100644 --- a/pkg/resolver/helm/render.go +++ b/pkg/resolver/helm/render.go @@ -228,23 +228,33 @@ func readChartContainedValuesFile(absPath string) ([]byte, error) { } // valuesFilePathForRead returns an absolute path to valuesFile only if it resolves under the chart directory. +// Both sides are evaluated through filepath.EvalSymlinks so a symlink inside the chart that points outside +// the chart tree is rejected; a lexical Rel/IsLocal check alone is unsafe because os.ReadFile follows symlinks. func valuesFilePathForRead(chartPath, valuesFile string) (string, error) { chartAbs, err := filepath.Abs(chartPath) if err != nil { return "", errors.Wrap(err, "resolve chart path") } chartAbs = filepath.Clean(chartAbs) + if ev, err := filepath.EvalSymlinks(chartAbs); err == nil { + chartAbs = filepath.Clean(ev) + } var p string if filepath.IsAbs(valuesFile) { p = filepath.Clean(valuesFile) } else { p = filepath.Clean(filepath.Join(chartAbs, valuesFile)) } - rel, err := filepath.Rel(chartAbs, p) + resolved, err := filepath.EvalSymlinks(p) + if err != nil { + return "", errors.Wrapf(err, "resolve values file %q", valuesFile) + } + resolved = filepath.Clean(resolved) + rel, err := filepath.Rel(chartAbs, resolved) if err != nil || !filepath.IsLocal(rel) { return "", errors.Errorf("values file %q is outside chart directory", valuesFile) } - return p, nil + return resolved, nil } func isHelmTestTemplate(path string) bool { diff --git a/pkg/resolver/helm/render_test.go b/pkg/resolver/helm/render_test.go index 291164343..6760f4fe9 100644 --- a/pkg/resolver/helm/render_test.go +++ b/pkg/resolver/helm/render_test.go @@ -129,6 +129,31 @@ func TestApplyValuesInline_CleanupRemovesTempFile(t *testing.T) { require.True(t, os.IsNotExist(statErr)) } +func TestValuesFilePathForRead_RejectsSymlinkEscape(t *testing.T) { + chartDir := t.TempDir() + outsideDir := t.TempDir() + secret := filepath.Join(outsideDir, "secret.yaml") + require.NoError(t, os.WriteFile(secret, []byte("password: hunter2\n"), 0o600)) + + link := filepath.Join(chartDir, "values.yaml") + require.NoError(t, os.Symlink(secret, link)) + + _, err := valuesFilePathForRead(chartDir, "values.yaml") + require.Error(t, err, "symlinked values file pointing outside the chart must be rejected") +} + +func TestValuesFilePathForRead_AcceptsSymlinkInsideChart(t *testing.T) { + chartDir := t.TempDir() + real := filepath.Join(chartDir, "real-values.yaml") + require.NoError(t, os.WriteFile(real, []byte("service:\n port: 80\n"), 0o600)) + link := filepath.Join(chartDir, "values.yaml") + require.NoError(t, os.Symlink(real, link)) + + p, err := valuesFilePathForRead(chartDir, "values.yaml") + require.NoError(t, err) + require.Equal(t, filepath.Base(real), filepath.Base(p), "symlinked values file inside chart should be accepted and resolved") +} + func TestRenderChart_PreservesExplicitReleaseName(t *testing.T) { ctx := context.Background() chartDir := testHelmFixtureChartDir(t) diff --git a/pkg/resolver/sandbox/bounded_fs.go b/pkg/resolver/sandbox/bounded_fs.go index cfabcbb11..6f45124a0 100644 --- a/pkg/resolver/sandbox/bounded_fs.go +++ b/pkg/resolver/sandbox/bounded_fs.go @@ -52,6 +52,12 @@ func (b *BoundedFS) checkCleaned(d filesys.ConfirmedDir, f string, resolveLeaf b full = filepath.Join(filepath.Clean(ev), filepath.Base(full)) } } + // Writes follow leaf symlinks at the OS layer, which would let a + // pre-existing or dangling symlink escape the sandbox. Reject any + // symlink leaf so the outside target is never created or overwritten. + if li, lerr := os.Lstat(full); lerr == nil && li.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("symlink leaf rejected for write: %s", full) + } } if !b.under(full) { return fmt.Errorf("path outside scan root: %s", full) diff --git a/pkg/resolver/sandbox/bounded_fs_test.go b/pkg/resolver/sandbox/bounded_fs_test.go index 8b0158e94..8fc2b41c3 100644 --- a/pkg/resolver/sandbox/bounded_fs_test.go +++ b/pkg/resolver/sandbox/bounded_fs_test.go @@ -54,3 +54,42 @@ func TestBoundedFS_RejectsWriteThroughSymlinkedParent(t *testing.T) { require.Error(t, statErr) require.True(t, os.IsNotExist(statErr)) } + +func TestBoundedFS_RejectsWriteThroughDanglingLeafSymlink(t *testing.T) { + root := t.TempDir() + outsideDir := t.TempDir() + outsideTarget := filepath.Join(outsideDir, "new.yaml") + link := filepath.Join(root, "link.yaml") + require.NoError(t, os.Symlink(outsideTarget, link)) + + fs, err := NewBoundedFS(root) + require.NoError(t, err) + + require.Error(t, fs.WriteFile(link, []byte("secret")), "write through dangling leaf symlink must be rejected") + _, statErr := os.Stat(outsideTarget) + require.Error(t, statErr) + require.True(t, os.IsNotExist(statErr), "outside target must not have been created") + + _, err = fs.Create(link) + require.Error(t, err, "create through dangling leaf symlink must be rejected") + _, statErr = os.Stat(outsideTarget) + require.Error(t, statErr) + require.True(t, os.IsNotExist(statErr)) +} + +func TestBoundedFS_RejectsWriteThroughExistingLeafSymlink(t *testing.T) { + root := t.TempDir() + outsideDir := t.TempDir() + outsideTarget := filepath.Join(outsideDir, "existing.yaml") + require.NoError(t, os.WriteFile(outsideTarget, []byte("original"), 0o600)) + link := filepath.Join(root, "link.yaml") + require.NoError(t, os.Symlink(outsideTarget, link)) + + fs, err := NewBoundedFS(root) + require.NoError(t, err) + + require.Error(t, fs.WriteFile(link, []byte("clobber")), "overwrite via leaf symlink must be rejected") + got, readErr := os.ReadFile(outsideTarget) + require.NoError(t, readErr) + require.Equal(t, "original", string(got), "outside target must not be overwritten") +} From 251b688fbc22ae55a3873eff4a0c6402bce8e5ee Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Mon, 27 Apr 2026 13:20:57 +0200 Subject: [PATCH 09/17] Codex comment: Confine Helm values files to the chart tree even without inline values, and route resolver-diagnostic messages into the storage dedup key --- pkg/kics/resolver_diagnostics.go | 3 ++ pkg/kics/resolver_diagnostics_test.go | 44 +++++++++++++++++++++++ pkg/resolver/helm/render.go | 48 +++++++++++++++++-------- pkg/resolver/helm/render_test.go | 51 +++++++++++++++++++++++---- 4 files changed, 126 insertions(+), 20 deletions(-) create mode 100644 pkg/kics/resolver_diagnostics_test.go diff --git a/pkg/kics/resolver_diagnostics.go b/pkg/kics/resolver_diagnostics.go index d9da828db..651a7d7e1 100644 --- a/pkg/kics/resolver_diagnostics.go +++ b/pkg/kics/resolver_diagnostics.go @@ -65,6 +65,9 @@ func (s *Service) saveResolverDiagnostics(ctx context.Context, scanID string, di SearchKey: "resolver_diagnostic." + d.QueryID, SearchLine: line, SearchValue: d.Message, + // MemoryStorage de-dupes on KeyActualValue but ignores SearchValue/Description, + // so distinct messages must reach the storage key through this field. + KeyActualValue: d.Message, }) } if len(vulns) == 0 { diff --git a/pkg/kics/resolver_diagnostics_test.go b/pkg/kics/resolver_diagnostics_test.go new file mode 100644 index 000000000..0905d5cac --- /dev/null +++ b/pkg/kics/resolver_diagnostics_test.go @@ -0,0 +1,44 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache-2.0 License. + * + * This product includes software developed at Datadog (https://www.datadoghq.com) Copyright 2024 Datadog, Inc. + */ +package kics + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/DataDog/datadog-iac-scanner/internal/storage" + "github.com/DataDog/datadog-iac-scanner/pkg/model" +) + +// Regression: diagnostics that share (QueryID, FilePath, Line) but have distinct messages +// must all survive MemoryStorage de-dup, which keys on KeyActualValue and ignores SearchValue. +func TestSaveResolverDiagnostics_DistinctMessagesArePreserved(t *testing.T) { + ctx := context.Background() + s := &Service{ + Storage: storage.NewMemoryStorage(), + ResolverDiagnostics: NewResolverDiagnosticsState(), + } + scanID := "scan-1" + diags := []model.ResolverDiagnostic{ + {QueryID: "q1", FilePath: "/repo/a.yaml", Line: 10, Message: "first"}, + {QueryID: "q1", FilePath: "/repo/a.yaml", Line: 10, Message: "second"}, + } + + require.NoError(t, s.saveResolverDiagnostics(ctx, scanID, diags)) + + got, err := s.Storage.GetVulnerabilities(ctx, scanID) + require.NoError(t, err) + require.Len(t, got, 2) + + messages := map[string]bool{} + for _, v := range got { + messages[v.SearchValue] = true + } + require.True(t, messages["first"], "first message must survive storage de-dup") + require.True(t, messages["second"], "second message must survive storage de-dup") +} diff --git a/pkg/resolver/helm/render.go b/pkg/resolver/helm/render.go index d500b67d1..dc5f55a91 100644 --- a/pkg/resolver/helm/render.go +++ b/pkg/resolver/helm/render.go @@ -78,14 +78,20 @@ func RenderChart(ctx context.Context, opts *RenderOptions) (*RenderedChart, erro } contextLogger := logger.FromContext(ctx) client := newClient(ctx, opts) + // Confine values files to the chart tree before they reach Helm; Helm's MergeValues + // would otherwise resolve relative paths against the scanner CWD and accept any absolute path. + normalizedFiles, err := normalizeValuesFiles(opts.ChartPath, opts.ValuesFiles) + if err != nil { + return nil, err + } valueOpts := &values.Options{ - ValueFiles: opts.ValuesFiles, + ValueFiles: normalizedFiles, Values: opts.SetValues, } cleanup := func() {} if len(opts.ValuesInline) > 0 { var err error - cleanup, err = applyValuesInline(ctx, valueOpts, opts) + cleanup, err = applyValuesInline(ctx, valueOpts, opts, normalizedFiles) if err != nil { return nil, err } @@ -116,12 +122,12 @@ func RenderChart(ctx context.Context, opts *RenderOptions) (*RenderedChart, erro return out, nil } -func applyValuesInline(ctx context.Context, valueOpts *values.Options, opts *RenderOptions) (func(), error) { +func applyValuesInline(ctx context.Context, valueOpts *values.Options, opts *RenderOptions, normalizedFiles []string) (func(), error) { if opts == nil { return func() {}, errors.New("helm RenderOptions is nil") } contextLogger := logger.FromContext(ctx) - inlineBytes, err := mergedValuesInlineBytes(opts) + inlineBytes, err := mergedValuesInlineBytes(opts, normalizedFiles) if err != nil { return func() {}, err } @@ -149,7 +155,7 @@ func applyValuesInline(ctx context.Context, valueOpts *values.Options, opts *Ren }, nil } -func mergedValuesInlineBytes(opts *RenderOptions) ([]byte, error) { +func mergedValuesInlineBytes(opts *RenderOptions, normalizedFiles []string) ([]byte, error) { if opts == nil { return nil, errors.New("helm RenderOptions is nil") } @@ -164,24 +170,21 @@ func mergedValuesInlineBytes(opts *RenderOptions) ([]byte, error) { default: return nil, errors.Errorf("invalid helm valuesMerge %q", opts.ValuesMerge) } - if len(opts.ValuesFiles) == 0 { + if len(normalizedFiles) == 0 { return kyaml.Marshal(opts.ValuesInline) } - baseNode, err := mergeHelmValueFilesToBaseNode(opts.ChartPath, opts.ValuesFiles) + baseNode, err := mergeHelmValueFilesToBaseNode(normalizedFiles) if err != nil { return nil, err } return mergeInlineYAMLWithBase(mergeMode, baseNode, opts.ValuesInline) } -// mergeHelmValueFilesToBaseNode merges valueFiles in order (later wins), after constraining paths to the chart tree. -func mergeHelmValueFilesToBaseNode(chartPath string, valueFiles []string) (*kyaml.RNode, error) { +// mergeHelmValueFilesToBaseNode merges valueFiles in order (later wins). +// Paths must already be confined to the chart tree by normalizeValuesFiles. +func mergeHelmValueFilesToBaseNode(valueFiles []string) (*kyaml.RNode, error) { var baseNode *kyaml.RNode - for i, valuesFile := range valueFiles { - safePath, err := valuesFilePathForRead(chartPath, valuesFile) - if err != nil { - return nil, err - } + for i, safePath := range valueFiles { baseBytes, err := readChartContainedValuesFile(safePath) if err != nil { return nil, errors.Wrap(err, "read helm base values file") @@ -227,6 +230,23 @@ func readChartContainedValuesFile(absPath string) ([]byte, error) { return os.ReadFile(absPath) } +// normalizeValuesFiles returns chart-tree-confined absolute paths for each entry, preserving order. +// Returns nil for an empty input so callers can treat it as "no values files". +func normalizeValuesFiles(chartPath string, files []string) ([]string, error) { + if len(files) == 0 { + return nil, nil + } + out := make([]string, 0, len(files)) + for _, f := range files { + safe, err := valuesFilePathForRead(chartPath, f) + if err != nil { + return nil, err + } + out = append(out, safe) + } + return out, nil +} + // valuesFilePathForRead returns an absolute path to valuesFile only if it resolves under the chart directory. // Both sides are evaluated through filepath.EvalSymlinks so a symlink inside the chart that points outside // the chart tree is rejected; a lexical Rel/IsLocal check alone is unsafe because os.ReadFile follows symlinks. diff --git a/pkg/resolver/helm/render_test.go b/pkg/resolver/helm/render_test.go index 6760f4fe9..17733261c 100644 --- a/pkg/resolver/helm/render_test.go +++ b/pkg/resolver/helm/render_test.go @@ -57,7 +57,7 @@ func TestMergedValuesInlineBytes_UsesAllValuesFilesBeforeInline(t *testing.T) { require.NoError(t, os.WriteFile(base, []byte("service:\n port: 80\n type: ClusterIP\n"), 0o600)) require.NoError(t, os.WriteFile(override, []byte("service:\n port: 8080\n"), 0o600)) - out, err := mergedValuesInlineBytes(&RenderOptions{ + opts := &RenderOptions{ ChartPath: dir, ValuesFiles: []string{base, override}, ValuesInline: map[string]interface{}{ @@ -66,7 +66,10 @@ func TestMergedValuesInlineBytes_UsesAllValuesFilesBeforeInline(t *testing.T) { }, }, ValuesMerge: valuesMergeOverride, - }) + } + normalizedFiles, err := normalizeValuesFiles(opts.ChartPath, opts.ValuesFiles) + require.NoError(t, err) + out, err := mergedValuesInlineBytes(opts, normalizedFiles) require.NoError(t, err) txt := string(out) require.Contains(t, txt, "port: 8080") @@ -82,7 +85,7 @@ func TestApplyValuesInline_ReplacesWholeValuesFileStack(t *testing.T) { require.NoError(t, os.WriteFile(override, []byte("service:\n port: 8080\n"), 0o600)) valueOpts := &values.Options{ValueFiles: []string{base, override}} - cleanup, err := applyValuesInline(ctx, valueOpts, &RenderOptions{ + opts := &RenderOptions{ ChartPath: dir, ValuesFiles: []string{base, override}, ValuesInline: map[string]interface{}{ @@ -91,7 +94,10 @@ func TestApplyValuesInline_ReplacesWholeValuesFileStack(t *testing.T) { }, }, ValuesMerge: valuesMergeOverride, - }) + } + normalizedFiles, err := normalizeValuesFiles(opts.ChartPath, opts.ValuesFiles) + require.NoError(t, err) + cleanup, err := applyValuesInline(ctx, valueOpts, opts, normalizedFiles) require.NoError(t, err) defer cleanup() require.Len(t, valueOpts.ValueFiles, 1) @@ -106,7 +112,7 @@ func TestApplyValuesInline_CleanupRemovesTempFile(t *testing.T) { require.NoError(t, os.WriteFile(base, []byte("service:\n port: 80\n"), 0o600)) valueOpts := &values.Options{ValueFiles: []string{base}} - cleanup, err := applyValuesInline(ctx, valueOpts, &RenderOptions{ + opts := &RenderOptions{ ChartPath: dir, ValuesFiles: []string{base}, ValuesInline: map[string]interface{}{ @@ -115,7 +121,10 @@ func TestApplyValuesInline_CleanupRemovesTempFile(t *testing.T) { }, }, ValuesMerge: valuesMergeOverride, - }) + } + normalizedFiles, err := normalizeValuesFiles(opts.ChartPath, opts.ValuesFiles) + require.NoError(t, err) + cleanup, err := applyValuesInline(ctx, valueOpts, opts, normalizedFiles) require.NoError(t, err) require.Len(t, valueOpts.ValueFiles, 1) tempPath := valueOpts.ValueFiles[0] @@ -129,6 +138,36 @@ func TestApplyValuesInline_CleanupRemovesTempFile(t *testing.T) { require.True(t, os.IsNotExist(statErr)) } +// Regression: a values file outside the chart tree must be rejected even when +// no ValuesInline is provided (the inline-only normalization path is bypassed). +func TestRenderChart_RejectsExternalValuesFileWithoutInline(t *testing.T) { + ctx := context.Background() + chartDir := testHelmFixtureChartDir(t) + outsideDir := t.TempDir() + external := filepath.Join(outsideDir, "external.yaml") + require.NoError(t, os.WriteFile(external, []byte("service:\n port: 9090\n"), 0o600)) + + _, err := RenderChart(ctx, &RenderOptions{ + ChartPath: chartDir, + ValuesFiles: []string{external}, + IncludeCRDs: true, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "outside chart directory") +} + +// Regression: a relative values file must resolve against ChartPath, not the scanner CWD. +func TestNormalizeValuesFiles_RelativeResolvesToChartPath(t *testing.T) { + chartDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(chartDir, "values.yaml"), []byte("a: 1\n"), 0o600)) + + out, err := normalizeValuesFiles(chartDir, []string{"values.yaml"}) + require.NoError(t, err) + require.Len(t, out, 1) + require.True(t, filepath.IsAbs(out[0])) + require.Equal(t, "values.yaml", filepath.Base(out[0])) +} + func TestValuesFilePathForRead_RejectsSymlinkEscape(t *testing.T) { chartDir := t.TempDir() outsideDir := t.TempDir() From 2636f652b4cacb29c7c98a37479a6133de0ffb88 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Wed, 29 Apr 2026 13:45:18 +0200 Subject: [PATCH 10/17] Add pkg/rootfile for os.OpenRoot-scoped file reads. --- pkg/rootfile/rootfile.go | 59 +++++++++++++++++++++++++++++++++++ pkg/rootfile/rootfile_test.go | 24 ++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 pkg/rootfile/rootfile.go create mode 100644 pkg/rootfile/rootfile_test.go diff --git a/pkg/rootfile/rootfile.go b/pkg/rootfile/rootfile.go new file mode 100644 index 000000000..31864acc7 --- /dev/null +++ b/pkg/rootfile/rootfile.go @@ -0,0 +1,59 @@ +// Package rootfile provides filesystem reads scoped with os.OpenRoot for safer path handling. +package rootfile + +import ( + "fmt" + "os" + "path/filepath" +) + +func openParentRoot(absPath string) (*os.Root, string, error) { + absPath = filepath.Clean(absPath) + dir := filepath.Dir(absPath) + rel, err := filepath.Rel(dir, absPath) + if err != nil { + return nil, "", err + } + if !filepath.IsLocal(rel) { + return nil, "", fmt.Errorf("path %q is not local under %q", absPath, dir) + } + r, err := os.OpenRoot(dir) + if err != nil { + return nil, "", err + } + return r, filepath.ToSlash(rel), nil +} + +// ReadFile reads absPath by opening filepath.Dir(absPath) with os.OpenRoot and reading the relative path. +func ReadFile(absPath string) ([]byte, error) { + r, rel, err := openParentRoot(absPath) + if err != nil { + return nil, err + } + data, rerr := r.ReadFile(rel) + cerr := r.Close() + if rerr != nil { + return nil, rerr + } + if cerr != nil { + return nil, cerr + } + return data, nil +} + +// Lstat returns file info for absPath using os.OpenRoot on the parent directory. +func Lstat(absPath string) (os.FileInfo, error) { + r, rel, err := openParentRoot(absPath) + if err != nil { + return nil, err + } + fi, rerr := r.Lstat(rel) + cerr := r.Close() + if rerr != nil { + return nil, rerr + } + if cerr != nil { + return nil, cerr + } + return fi, nil +} diff --git a/pkg/rootfile/rootfile_test.go b/pkg/rootfile/rootfile_test.go new file mode 100644 index 000000000..714a6ebae --- /dev/null +++ b/pkg/rootfile/rootfile_test.go @@ -0,0 +1,24 @@ +package rootfile + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestReadFile_roundTrip(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "a", "b.txt") + require.NoError(t, os.MkdirAll(filepath.Dir(p), 0o700)) + require.NoError(t, os.WriteFile(p, []byte("hello"), 0o600)) + + got, err := ReadFile(p) + require.NoError(t, err) + require.Equal(t, "hello", string(got)) + + fi, err := Lstat(p) + require.NoError(t, err) + require.False(t, fi.IsDir()) +} From 5cab6e4c535d6ca9546da5329fa3b19b0daed3f5 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Wed, 29 Apr 2026 13:45:25 +0200 Subject: [PATCH 11/17] Add Kustomize detection, stdlib-only staging primitives, and shared path-confinement helper. --- go.mod | 4 +- pkg/resolver/kustomize/copytree.go | 91 +++++++++++++++++++++++++ pkg/resolver/kustomize/copytree_test.go | 28 ++++++++ pkg/resolver/kustomize/detect.go | 31 +++++++++ pkg/resolver/kustomize/namehash.go | 33 +++++++++ pkg/resolver/kustomize/paths.go | 14 ++++ 6 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 pkg/resolver/kustomize/copytree.go create mode 100644 pkg/resolver/kustomize/copytree_test.go create mode 100644 pkg/resolver/kustomize/detect.go create mode 100644 pkg/resolver/kustomize/namehash.go create mode 100644 pkg/resolver/kustomize/paths.go diff --git a/go.mod b/go.mod index 93a72a8d6..d8af3a658 100644 --- a/go.mod +++ b/go.mod @@ -44,6 +44,8 @@ require ( k8s.io/client-go v0.35.1 mvdan.cc/sh/v3 v3.12.0 sigs.k8s.io/controller-runtime v0.23.1 + sigs.k8s.io/kustomize/api v0.21.1 + sigs.k8s.io/kustomize/kyaml v0.21.1 ) require ( @@ -167,8 +169,6 @@ require ( k8s.io/component-base v0.35.1 // indirect k8s.io/kubectl v0.35.1 // indirect oras.land/oras-go/v2 v2.6.0 // indirect - sigs.k8s.io/kustomize/api v0.21.1 // indirect - sigs.k8s.io/kustomize/kyaml v0.21.1 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.2 // indirect ) diff --git a/pkg/resolver/kustomize/copytree.go b/pkg/resolver/kustomize/copytree.go new file mode 100644 index 000000000..47151be2c --- /dev/null +++ b/pkg/resolver/kustomize/copytree.go @@ -0,0 +1,91 @@ +package kustomize + +import ( + "errors" + "os" + "path/filepath" + "strings" +) + +const ( + dirPermCopyTree = 0o700 + filePermCopyTree = 0o600 + parentDirPrefix = ".." + string(filepath.Separator) + relDotDot = ".." +) + +func mkdirParentInRoot(dstR *os.Root, rel string) error { + parent := filepath.Dir(rel) + if parent == "." || parent == "" { + return nil + } + parentSlash := filepath.ToSlash(filepath.Clean(parent)) + if parentSlash == "." { + return nil + } + return dstR.MkdirAll(parentSlash, dirPermCopyTree) +} + +func copyOneRelFile(dstR, srcR *os.Root, srcRoot, rel string) error { + rel = filepath.Clean(rel) + if rel == "." || !filepath.IsLocal(rel) || strings.HasPrefix(rel, parentDirPrefix) || rel == relDotDot { + return nil + } + joined := filepath.Join(srcRoot, rel) + if !isUnderRoot(joined, srcRoot) { + return nil + } + relSlash := filepath.ToSlash(rel) + fi, err := srcR.Lstat(relSlash) + if err != nil || fi.Mode()&os.ModeSymlink != 0 || fi.IsDir() { + return nil + } + data, err := srcR.ReadFile(relSlash) + if err != nil { + return err + } + if err := mkdirParentInRoot(dstR, rel); err != nil { + return err + } + df, err := dstR.OpenFile(relSlash, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, filePermCopyTree) + if err != nil { + return err + } + _, werr := df.Write(data) + cerr := df.Close() + return errors.Join(werr, cerr) +} + +// CopyRepoRelativeFilesNoSymlinks copies relFiles from srcRoot to dstRoot by relative path; skips symlinks and out-of-root paths. +func CopyRepoRelativeFilesNoSymlinks(dstRoot, srcRoot string, relFiles []string) (err error) { + srcRoot = filepath.Clean(srcRoot) + dstRoot = filepath.Clean(dstRoot) + if mkerr := os.MkdirAll(dstRoot, dirPermCopyTree); mkerr != nil { + return mkerr + } + srcR, err := os.OpenRoot(srcRoot) + if err != nil { + return err + } + defer func() { + if cerr := srcR.Close(); cerr != nil { + err = errors.Join(err, cerr) + } + }() + dstR, err := os.OpenRoot(dstRoot) + if err != nil { + return err + } + defer func() { + if cerr := dstR.Close(); cerr != nil { + err = errors.Join(err, cerr) + } + }() + + for _, rel := range relFiles { + if cerr := copyOneRelFile(dstR, srcR, srcRoot, rel); cerr != nil { + return cerr + } + } + return err +} diff --git a/pkg/resolver/kustomize/copytree_test.go b/pkg/resolver/kustomize/copytree_test.go new file mode 100644 index 000000000..080bec026 --- /dev/null +++ b/pkg/resolver/kustomize/copytree_test.go @@ -0,0 +1,28 @@ +package kustomize + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCopyRepoRelativeFilesNoSymlinks_skipsSymlinks(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + secret := filepath.Join(t.TempDir(), "secret.txt") + require.NoError(t, os.WriteFile(secret, []byte("secret"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(src, "ok.txt"), []byte("ok"), 0o600)) + require.NoError(t, os.Symlink(secret, filepath.Join(src, "leak"))) + + require.NoError(t, CopyRepoRelativeFilesNoSymlinks(dst, src, []string{"ok.txt", "leak"})) + + got, err := os.ReadFile(filepath.Join(dst, "ok.txt")) + require.NoError(t, err) + require.Equal(t, "ok", string(got)) + _, err = os.Stat(filepath.Join(dst, "leak")) + require.Error(t, err) + require.True(t, errors.Is(err, os.ErrNotExist), "symlink must not be copied") +} diff --git a/pkg/resolver/kustomize/detect.go b/pkg/resolver/kustomize/detect.go new file mode 100644 index 000000000..9fa7f5db3 --- /dev/null +++ b/pkg/resolver/kustomize/detect.go @@ -0,0 +1,31 @@ +package kustomize + +import ( + "os" + "path/filepath" + + "github.com/DataDog/datadog-iac-scanner/pkg/model" +) + +var kustomizationNames = []string{"kustomization.yaml", "kustomization.yml", "Kustomization"} + +// IsKustomizationEntryFile reports whether path is a kustomization entry filename. +func IsKustomizationEntryFile(path string) bool { + base := filepath.Base(path) + for _, n := range kustomizationNames { + if base == n { + return true + } + } + return false +} + +// Detect returns KindKUSTOMIZE when the directory contains a kustomization entry file. +func Detect(dir string) (model.FileKind, bool) { + for _, n := range kustomizationNames { + if _, err := os.Stat(filepath.Join(dir, n)); err == nil { + return model.KindKUSTOMIZE, true + } + } + return model.KindCOMMON, false +} diff --git a/pkg/resolver/kustomize/namehash.go b/pkg/resolver/kustomize/namehash.go new file mode 100644 index 000000000..87a685024 --- /dev/null +++ b/pkg/resolver/kustomize/namehash.go @@ -0,0 +1,33 @@ +package kustomize + +import ( + "regexp" + + "sigs.k8s.io/kustomize/api/resource" +) + +// Default kustomize content-hash suffix length is 10 alphanumeric characters. +var kustomizeNameHashSuffix = regexp.MustCompile(`-[a-z0-9]{10}$`) + +func stripKustomizeNameHashSuffix(name string) string { + return kustomizeNameHashSuffix.ReplaceAllString(name, "") +} + +// generatorResourceName returns the stable id for generator-produced resources (pre-hash-suffix). +// OrgId().Name may still include the content hash; strip a trailing kustomize hash when present. +func generatorResourceName(res *resource.Resource) string { + if res == nil { + return "" + } + rendered := res.GetName() + if s := stripKustomizeNameHashSuffix(rendered); s != "" && s != rendered { + return s + } + if n := res.OrgId().Name; n != "" { + if s := stripKustomizeNameHashSuffix(n); s != "" && s != n { + return s + } + return n + } + return rendered +} diff --git a/pkg/resolver/kustomize/paths.go b/pkg/resolver/kustomize/paths.go new file mode 100644 index 000000000..40923abe3 --- /dev/null +++ b/pkg/resolver/kustomize/paths.go @@ -0,0 +1,14 @@ +package kustomize + +import ( + "path/filepath" + "strings" +) + +// isUnderRoot reports whether path is root itself or strictly under root. +// Used to confine reads/writes to the configured repo or scratch trees. +func isUnderRoot(path, root string) bool { + cp := filepath.Clean(path) + cr := filepath.Clean(root) + return cp == cr || strings.HasPrefix(cp, cr+string(filepath.Separator)) +} From ef62010707a906f8e9503fc0362df39ec0e6a6b4 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Wed, 29 Apr 2026 13:45:32 +0200 Subject: [PATCH 12/17] Add Kustomize discovery, build-metadata staging, origin/transformation parsing, and remote-ref recognition predicates. --- pkg/resolver/kustomize/attribute.go | 217 ++++++++++ pkg/resolver/kustomize/buildmetadata.go | 95 +++++ pkg/resolver/kustomize/buildmetadata_test.go | 60 +++ pkg/resolver/kustomize/discover.go | 402 ++++++++++++++++++ .../kustomize/discover_buildmetadata_test.go | 80 ++++ .../kustomize/generator_origin_test.go | 48 +++ .../kustomize/konfig_constants_test.go | 12 + pkg/resolver/kustomize/remote_ref.go | 69 +++ .../kustomize/transformations_test.go | 80 ++++ 9 files changed, 1063 insertions(+) create mode 100644 pkg/resolver/kustomize/attribute.go create mode 100644 pkg/resolver/kustomize/buildmetadata.go create mode 100644 pkg/resolver/kustomize/buildmetadata_test.go create mode 100644 pkg/resolver/kustomize/discover.go create mode 100644 pkg/resolver/kustomize/discover_buildmetadata_test.go create mode 100644 pkg/resolver/kustomize/generator_origin_test.go create mode 100644 pkg/resolver/kustomize/konfig_constants_test.go create mode 100644 pkg/resolver/kustomize/remote_ref.go create mode 100644 pkg/resolver/kustomize/transformations_test.go diff --git a/pkg/resolver/kustomize/attribute.go b/pkg/resolver/kustomize/attribute.go new file mode 100644 index 000000000..ef224b4aa --- /dev/null +++ b/pkg/resolver/kustomize/attribute.go @@ -0,0 +1,217 @@ +package kustomize + +import ( + "os" + "path/filepath" + "strings" + + "github.com/DataDog/datadog-iac-scanner/pkg/model" + "sigs.k8s.io/kustomize/api/konfig" + "sigs.k8s.io/kustomize/api/resource" +) + +// TransformerAnnotationKey is the alpha transformation-chain annotation (must match upstream; see konfig_constants_test). +const TransformerAnnotationKey = "alpha.config.kubernetes.io/transformations" + +func helmInflatedOrigin(res *resource.Resource, kustAbs string) *model.KustomizeOrigin { + ann := res.GetAnnotations() + if ann == nil { + return nil + } + if _, ok := ann[konfig.HelmGeneratedAnnotation]; !ok { + return nil + } + return &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginHelmInflated, + GeneratorConfigFile: kustAbs, + ResourceGVK: res.GetGvk().String(), + ResourceName: res.GetName(), + } +} + +func appendTransformationFromOrigin( + chain []model.KustomizeTransformation, + t *resource.Origin, + kustomizationDir, origSourceRepo string, +) []model.KustomizeTransformation { + if t == nil { + return chain + } + configuredIn := "" + if t.ConfiguredIn != "" { + configuredIn = t.ConfiguredIn + if origSourceRepo != "" { + configuredIn = joinRepoURL(origSourceRepo, configuredIn) + } else if !filepath.IsAbs(configuredIn) { + configuredIn = filepath.Join(kustomizationDir, filepath.Clean(configuredIn)) + } + } + transformerPath := resolveOriginPath(kustomizationDir, origSourceRepo, t.Path) + if configuredIn != "" && t.Path != "" && !looksLikeRemotePath(configuredIn) { + transformerPath = filepath.Join(filepath.Dir(configuredIn), filepath.Clean(t.Path)) + } else if configuredIn != "" && t.Path != "" && looksLikeRemotePath(configuredIn) { + transformerPath = joinRepoURL(dirURL(configuredIn), t.Path) + } + return append(chain, model.KustomizeTransformation{ + TransformerPath: cleanLocalPath(transformerPath), + ConfiguredIn: configuredIn, + FieldPath: t.Path, + }) +} + +func transformerOrigin(res *resource.Resource, kustomizationDir, kustAbs string) *model.KustomizeOrigin { + trans, err := res.GetTransformations() + if err != nil || len(trans) == 0 { + return nil + } + origSourceFile, origSourceRepo, origSourceRef := "", "", "" + if origin, originErr := res.GetOrigin(); originErr == nil && origin != nil && origin.Path != "" { + origSourceFile = resolveOriginPath(kustomizationDir, origin.Repo, origin.Path) + origSourceRepo = origin.Repo + origSourceRef = origin.Ref + } + var chain []model.KustomizeTransformation + for _, t := range trans { + chain = appendTransformationFromOrigin(chain, t, kustomizationDir, origSourceRepo) + } + return &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginTransformer, + GeneratorConfigFile: kustAbs, + Transformations: chain, + OriginalSourceFile: origSourceFile, + OriginalSourceRepo: origSourceRepo, + OriginalSourceRef: origSourceRef, + ResourceGVK: res.GetGvk().String(), + ResourceName: res.GetName(), + } +} + +// OriginFromResource builds KustomizeOrigin from kustomize API resource metadata. +func OriginFromResource(res *resource.Resource, kustomizationDir string) *model.KustomizeOrigin { + if res == nil { + return nil + } + kustFile := kustomizationEntryFile(kustomizationDir) + kustAbs := filepath.Join(kustomizationDir, kustFile) + + if o := helmInflatedOrigin(res, kustAbs); o != nil { + return o + } + if o := transformerOrigin(res, kustomizationDir, kustAbs); o != nil { + return o + } + + origin, err := res.GetOrigin() + if err != nil || origin == nil { + return &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginDirect, + GeneratorConfigFile: kustAbs, + ResourceGVK: res.GetGvk().String(), + ResourceName: res.GetName(), + } + } + + cbKind := origin.ConfiguredBy.GetKind() + cbName := origin.ConfiguredBy.GetName() + if origin.ConfiguredIn != "" && isGeneratorConfiguredKind(cbKind) { + cfg := origin.ConfiguredIn + if !filepath.IsAbs(cfg) { + cfg = filepath.Join(kustomizationDir, cfg) + } + return &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginGenerator, + GeneratorKind: cbKind, + ConfiguredByKind: cbKind, + ConfiguredByName: cbName, + GeneratorConfigFile: cfg, + ResourceGVK: res.GetGvk().String(), + ResourceName: generatorResourceName(res), + } + } + + if origin.Path != "" { + sf := resolveOriginPath(kustomizationDir, origin.Repo, origin.Path) + return &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginDirect, + SourceFile: sf, + SourceRepo: origin.Repo, + SourceRef: origin.Ref, + ResourceGVK: res.GetGvk().String(), + ResourceName: res.GetName(), + GeneratorConfigFile: kustAbs, + } + } + + return &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginDirect, + GeneratorConfigFile: kustAbs, + ResourceGVK: res.GetGvk().String(), + ResourceName: res.GetName(), + } +} + +func resolveOriginPath(kustomizationDir, repo, path string) string { + if path == "" { + return "" + } + if repo != "" { + return joinRepoURL(repo, path) + } + if filepath.IsAbs(path) { + return path + } + return filepath.Join(kustomizationDir, path) +} + +func looksLikeRemotePath(path string) bool { + return strings.Contains(path, "://") +} + +func joinRepoURL(base, rel string) string { + if base == "" { + return rel + } + if rel == "" { + return base + } + if looksLikeRemotePath(rel) { + return rel + } + return strings.TrimRight(base, "/") + "/" + strings.TrimLeft(filepath.ToSlash(rel), "/") +} + +func dirURL(path string) string { + if !looksLikeRemotePath(path) { + return filepath.Dir(path) + } + path = strings.TrimRight(path, "/") + idx := strings.LastIndex(path, "/") + if idx < 0 { + return path + } + return path[:idx] +} + +func cleanLocalPath(path string) string { + if looksLikeRemotePath(path) { + return path + } + return filepath.Clean(path) +} + +func isGeneratorConfiguredKind(kind string) bool { + k := strings.ToLower(kind) + if k == "" { + return false + } + return strings.Contains(k, "generator") +} + +func kustomizationEntryFile(dir string) string { + for _, n := range kustomizationNames { + if _, err := os.Stat(filepath.Join(dir, n)); err == nil { + return n + } + } + return "kustomization.yaml" +} diff --git a/pkg/resolver/kustomize/buildmetadata.go b/pkg/resolver/kustomize/buildmetadata.go new file mode 100644 index 000000000..1894c8328 --- /dev/null +++ b/pkg/resolver/kustomize/buildmetadata.go @@ -0,0 +1,95 @@ +package kustomize + +import ( + "bytes" + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/DataDog/datadog-iac-scanner/pkg/rootfile" + "gopkg.in/yaml.v3" +) + +const buildMetadataFilePerm = 0o600 + +func buildMetadataSupplementsNeeded(data []byte) (bool, error) { + var doc map[string]interface{} + if err := yaml.Unmarshal(data, &doc); err != nil { + return false, err + } + meta, ok := doc["buildMetadata"].([]interface{}) + if !ok || len(meta) == 0 { + return true, nil + } + has := func(want string) bool { + for _, m := range meta { + if s, ok := m.(string); ok && s == want { + return true + } + } + return false + } + return !has("originAnnotations") || !has("transformerAnnotations"), nil +} + +func ensureBuildMetadataEntries(doc map[string]interface{}) { + meta, ok := doc["buildMetadata"].([]interface{}) + if !ok || len(meta) == 0 { + doc["buildMetadata"] = []interface{}{"originAnnotations", "transformerAnnotations"} + return + } + has := func(want string) bool { + for _, m := range meta { + if s, ok := m.(string); ok && s == want { + return true + } + } + return false + } + if !has("originAnnotations") { + meta = append(meta, "originAnnotations") + } + if !has("transformerAnnotations") { + meta = append(meta, "transformerAnnotations") + } + doc["buildMetadata"] = meta +} + +// ensureBuildMetadataIfNeeded adds originAnnotations / transformerAnnotations when missing; writes only if bytes change. +func ensureBuildMetadataIfNeeded(dir string) error { + kf := kustomizationEntryFile(dir) + cleanDir := filepath.Clean(dir) + p := filepath.Join(cleanDir, kf) + p = filepath.Clean(p) + if !isUnderRoot(p, cleanDir) { + return fmt.Errorf("invalid kustomization path under %q", cleanDir) + } + data, err := rootfile.ReadFile(p) + if err != nil { + return err + } + need, err := buildMetadataSupplementsNeeded(data) + if err != nil || !need { + return err + } + var doc map[string]interface{} + if err := yaml.Unmarshal(data, &doc); err != nil { + return err + } + ensureBuildMetadataEntries(doc) + b, err := yaml.Marshal(doc) + if err != nil { + return err + } + if bytes.Equal(data, b) { + return nil + } + root, err := os.OpenRoot(cleanDir) + if err != nil { + return err + } + werr := root.WriteFile(filepath.ToSlash(kf), b, buildMetadataFilePerm) + cerr := root.Close() + return errors.Join(werr, cerr) +} diff --git a/pkg/resolver/kustomize/buildmetadata_test.go b/pkg/resolver/kustomize/buildmetadata_test.go new file mode 100644 index 000000000..802ea4acf --- /dev/null +++ b/pkg/resolver/kustomize/buildmetadata_test.go @@ -0,0 +1,60 @@ +package kustomize + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestBuildMetadataSupplementsNeeded_trueWithoutTransformerAnnotations(t *testing.T) { + raw := `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +buildMetadata: +- originAnnotations +resources: [] +` + need, err := buildMetadataSupplementsNeeded([]byte(raw)) + require.NoError(t, err) + require.True(t, need) +} + +func TestBuildMetadataSupplementsNeeded_falseWhenComplete(t *testing.T) { + raw := `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +buildMetadata: +- originAnnotations +- transformerAnnotations +resources: [] +` + need, err := buildMetadataSupplementsNeeded([]byte(raw)) + require.NoError(t, err) + require.False(t, need) +} + +func TestResolve_doesNotRewriteUserKustomizationWhenMetadataComplete(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "cm.yaml"), []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: x\ndata: {}\n"), 0o600)) + kust := `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +buildMetadata: +- originAnnotations +- transformerAnnotations +resources: +- cm.yaml +` + path := filepath.Join(dir, "kustomization.yaml") + require.NoError(t, os.WriteFile(path, []byte(kust), 0o600)) + before, err := os.ReadFile(path) + require.NoError(t, err) + + r := NewResolver(Options{RepoRoot: dir, AllowHelmInflation: false}) + _, err = r.Resolve(ctx, dir) + require.NoError(t, err) + after, err := os.ReadFile(path) + require.NoError(t, err) + require.Equal(t, before, after, "scanner must not rewrite the user's kustomization on disk") +} diff --git a/pkg/resolver/kustomize/discover.go b/pkg/resolver/kustomize/discover.go new file mode 100644 index 000000000..a45a725b6 --- /dev/null +++ b/pkg/resolver/kustomize/discover.go @@ -0,0 +1,402 @@ +package kustomize + +import ( + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/DataDog/datadog-iac-scanner/pkg/rootfile" + "gopkg.in/yaml.v3" +) + +// TransitiveLocalPaths returns local file paths referenced by a kustomization (best-effort for Excluded). +func TransitiveLocalPaths(root string) ([]string, error) { + seen := map[string]struct{}{} + var out []string + addAbs := func(abs string) { + abs = filepath.Clean(abs) + if _, ok := seen[abs]; ok { + return + } + seen[abs] = struct{}{} + out = append(out, abs) + } + err := WalkLocalKustomizations(root, func(kustPath string, raw []byte) error { + addAbs(kustPath) + kustDir := filepath.Dir(kustPath) + var doc map[string]interface{} + if err := yaml.Unmarshal(raw, &doc); err != nil { + return err + } + add := func(p string) { + if p == "" || strings.Contains(p, "://") { + return + } + addAbs(filepath.Join(kustDir, filepath.Clean(p))) + } + appendDeclaredLocalPathStrings(doc, add) + return nil + }) + if err != nil { + return nil, err + } + return out, nil +} + +// TransitiveRelativeLocalPaths lists relative local refs from the kustomization graph (no abs paths: keeps inference in-tree). +func TransitiveRelativeLocalPaths(root string) ([]string, error) { + root = filepath.Clean(root) + seen := map[string]struct{}{root: {}} + out := []string{root} + err := WalkLocalKustomizations(root, func(kustPath string, raw []byte) error { + kustDir := filepath.Dir(kustPath) + if _, ok := seen[kustDir]; !ok { + seen[kustDir] = struct{}{} + out = append(out, kustDir) + } + var doc map[string]interface{} + if err := yaml.Unmarshal(raw, &doc); err != nil { + return err + } + add := func(p string) { + if p == "" || strings.Contains(p, "://") || filepath.IsAbs(p) { + return + } + abs := filepath.Clean(filepath.Join(kustDir, p)) + if _, ok := seen[abs]; ok { + return + } + seen[abs] = struct{}{} + out = append(out, abs) + } + appendDeclaredLocalPathStrings(doc, add) + return nil + }) + if err != nil { + return nil, err + } + return out, nil +} + +const defaultHelmChartHome = "charts" + +func appendFromMixedStringOrPathList(v interface{}, add func(string)) { + t, ok := v.([]interface{}) + if !ok { + return + } + for _, item := range t { + switch x := item.(type) { + case string: + add(x) + case map[string]interface{}: + if p, ok := x["path"].(string); ok { + add(p) + } + } + } +} + +func appendHelmChartsDeclaredPaths(doc map[string]interface{}, add func(string)) { + list, ok := doc["helmCharts"].([]interface{}) + if !ok { + return + } + chartHome := defaultHelmChartHome + if g, ok := doc["helmGlobals"].(map[string]interface{}); ok { + if ch, ok := g["chartHome"].(string); ok && ch != "" { + chartHome = ch + } + } + for _, item := range list { + m, ok := item.(map[string]interface{}) + if !ok { + continue + } + if name, ok := m["name"].(string); ok && name != "" { + add(filepath.Join(chartHome, name)) + } + if vf, ok := m["valuesFile"].(string); ok { + add(vf) + } + if av, ok := m["additionalValuesFiles"].([]interface{}); ok { + for _, it := range av { + if s, ok := it.(string); ok { + add(s) + } + } + } + } +} + +func appendPatchesJSON6902Paths(doc map[string]interface{}, add func(string)) { + list, ok := doc["patchesJson6902"].([]interface{}) + if !ok { + return + } + for _, item := range list { + m, ok := item.(map[string]interface{}) + if !ok { + continue + } + if p, ok := m["path"].(string); ok { + add(p) + } + } +} + +func appendGeneratorFilePaths(doc map[string]interface{}, add func(string)) { + for _, k := range []string{"configMapGenerator", "secretGenerator"} { + v, ok := doc[k] + if !ok { + continue + } + list, ok := v.([]interface{}) + if !ok { + continue + } + for _, item := range list { + m, ok := item.(map[string]interface{}) + if !ok { + continue + } + if files, ok := m["files"].([]interface{}); ok { + for _, f := range files { + if s, ok := f.(string); ok { + parts := strings.SplitN(s, "=", 2) + add(parts[len(parts)-1]) + } + } + } + if envs, ok := m["envs"].([]interface{}); ok { + for _, f := range envs { + if s, ok := f.(string); ok { + add(s) + } + } + } + } + } +} + +// appendDeclaredLocalPathStrings calls add for each local path in doc (resources, patches, generators, …). +// Paths are as written in YAML, relative to the kustomization directory. +func appendDeclaredLocalPathStrings(doc map[string]interface{}, add func(string)) { + for _, k := range []string{ + "resources", "components", "bases", + "patchesStrategicMerge", "crds", "configurations", + } { + if v, ok := doc[k]; ok { + appendFromMixedStringOrPathList(v, add) + } + } + if v, ok := doc["replacements"]; ok { + collectReplacementFileRefs(v, add) + } + for _, k := range []string{"patches", "generators", "transformers", "validators"} { + if v, ok := doc[k]; ok { + appendFromMixedStringOrPathList(v, add) + } + } + if om, ok := doc["openapi"].(map[string]interface{}); ok { + if p, ok := om["path"].(string); ok { + add(p) + } + } + appendHelmChartsDeclaredPaths(doc, add) + appendPatchesJSON6902Paths(doc, add) + appendGeneratorFilePaths(doc, add) +} + +// BuildMetadataStageRelPaths lists repo-relative files to copy into scratch for buildMetadata +// (full kustom tree + declared locals under repoRoot). +func BuildMetadataStageRelPaths(repoRoot, kustomRoot string) ([]string, error) { + repoRoot = filepath.Clean(repoRoot) + kustomRoot = filepath.Clean(kustomRoot) + set := make(map[string]struct{}) + if err := WalkLocalKustomizations(kustomRoot, func(kustPath string, raw []byte) error { + kustDir := filepath.Dir(kustPath) + if err := treeFilesUnderIntoRelSet(repoRoot, kustDir, set); err != nil { + return err + } + var doc map[string]interface{} + if err := yaml.Unmarshal(raw, &doc); err != nil { + return err + } + add := func(p string) { + if p == "" || strings.Contains(p, "://") { + return + } + abs := filepath.Join(kustDir, filepath.Clean(p)) + _ = stageAbsPathFilesIntoRelSet(repoRoot, abs, set) + } + appendDeclaredLocalPathStrings(doc, add) + return nil + }); err != nil { + return nil, err + } + + out := make([]string, 0, len(set)) + for r := range set { + out = append(out, r) + } + sort.Strings(out) + return out, nil +} + +// StageRelPathsForKustomRoot is repo-relative paths to stage one kustom root in scratch (delegates to BuildMetadataStageRelPaths). +func StageRelPathsForKustomRoot(repoRoot, kustomRoot string) ([]string, error) { + return BuildMetadataStageRelPaths(repoRoot, kustomRoot) +} + +func kustomBaseChildDirsFromDoc(doc map[string]interface{}, dir string) []string { + var next []string + addRef := func(p string) { + p = strings.TrimSpace(p) + if p == "" || isRemoteKustomizeRef(p) { + return + } + abs := filepath.Clean(filepath.Join(dir, p)) + if st, err := os.Lstat(abs); err == nil && st.Mode()&fs.ModeSymlink == 0 && st.IsDir() { + if _, ok := Detect(abs); ok { + next = append(next, abs) + } + } + } + for _, key := range []string{"resources", "components", "bases"} { + v, ok := doc[key] + if !ok { + continue + } + list, ok := v.([]interface{}) + if !ok { + continue + } + for _, item := range list { + switch x := item.(type) { + case string: + addRef(x) + case map[string]interface{}: + if p, ok := x["path"].(string); ok { + addRef(p) + } + } + } + } + return next +} + +// WalkLocalKustomizations walks local bases/components/resources (no remote URLs). +func WalkLocalKustomizations(root string, visit func(kustPath string, raw []byte) error) error { + root = filepath.Clean(root) + type item struct { + dir string + } + queue := []item{{dir: root}} + seen := map[string]struct{}{} + + for len(queue) > 0 { + cur := queue[0] + queue = queue[1:] + dir := filepath.Clean(cur.dir) + if _, ok := seen[dir]; ok { + continue + } + seen[dir] = struct{}{} + + kf := kustomizationEntryFile(dir) + kustPath := filepath.Join(dir, kf) + raw, err := rootfile.ReadFile(kustPath) + if err != nil { + return err + } + if err := visit(kustPath, raw); err != nil { + return err + } + + var doc map[string]interface{} + if err := yaml.Unmarshal(raw, &doc); err != nil { + return err + } + for _, d := range kustomBaseChildDirsFromDoc(doc, dir) { + queue = append(queue, item{dir: d}) + } + } + return nil +} + +func treeFilesUnderIntoRelSet(repoRoot, root string, set map[string]struct{}) error { + return filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.Type()&fs.ModeSymlink != 0 { + if d.IsDir() { + return filepath.SkipDir + } + return nil + } + if d.IsDir() { + return nil + } + rel, err := filepath.Rel(repoRoot, path) + if err != nil { + return nil + } + if rel == relDotDot || strings.HasPrefix(rel, parentDirPrefix) { + return nil + } + set[rel] = struct{}{} + return nil + }) +} + +func stageAbsPathFilesIntoRelSet(repoRoot, abs string, set map[string]struct{}) error { + abs = filepath.Clean(abs) + rel, err := filepath.Rel(filepath.Clean(repoRoot), abs) + if err != nil { + return err + } + if rel == relDotDot || strings.HasPrefix(rel, parentDirPrefix) { + return nil + } + fi, err := os.Lstat(abs) + if err != nil { + return nil + } + if fi.Mode()&fs.ModeSymlink != 0 { + return nil + } + if fi.IsDir() { + return treeFilesUnderIntoRelSet(repoRoot, abs, set) + } + set[rel] = struct{}{} + return nil +} + +// collectReplacementFileRefs feeds add with replacement file paths only (top-level list strings and map `path`; ignores other scalars). +func collectReplacementFileRefs(v interface{}, add func(string)) { + collectReplacementFileRefsRec(v, add, true) +} + +func collectReplacementFileRefsRec(v interface{}, add func(string), topLevel bool) { + switch t := v.(type) { + case map[string]interface{}: + if p, ok := t["path"].(string); ok { + add(p) + } + case []interface{}: + for _, it := range t { + switch x := it.(type) { + case string: + if topLevel { + add(x) + } + default: + collectReplacementFileRefsRec(x, add, false) + } + } + } +} diff --git a/pkg/resolver/kustomize/discover_buildmetadata_test.go b/pkg/resolver/kustomize/discover_buildmetadata_test.go new file mode 100644 index 000000000..b6709fc9b --- /dev/null +++ b/pkg/resolver/kustomize/discover_buildmetadata_test.go @@ -0,0 +1,80 @@ +package kustomize + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestBuildMetadataStageRelPaths_includesParentDirectoryRefs(t *testing.T) { + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(base, "cm.yaml"), []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: x\ndata:\n k: v\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +`), 0o600)) + + rels, err := BuildMetadataStageRelPaths(repo, overlay) + require.NoError(t, err) + require.Contains(t, rels, filepath.Join("overlay", "kustomization.yaml")) + require.Contains(t, rels, filepath.Join("base", "cm.yaml")) +} + +func TestWalkLocalKustomizations_visitsNestedBases(t *testing.T) { + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(base, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: [] +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +`), 0o600)) + + var visited []string + err := WalkLocalKustomizations(overlay, func(kustPath string, _ []byte) error { + visited = append(visited, filepath.Clean(kustPath)) + return nil + }) + require.NoError(t, err) + require.Contains(t, visited, filepath.Join(overlay, "kustomization.yaml")) + require.Contains(t, visited, filepath.Join(base, "kustomization.yaml")) +} + +func TestBuildMetadataStageRelPaths_includesNestedChildParentRefs(t *testing.T) { + repo := t.TempDir() + common := filepath.Join(repo, "common") + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(common, 0o700)) + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(common, "shared.yaml"), []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: shared\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(base, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../common/shared.yaml +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +`), 0o600)) + + rels, err := BuildMetadataStageRelPaths(repo, overlay) + require.NoError(t, err) + require.Contains(t, rels, filepath.Join("base", "kustomization.yaml")) + require.Contains(t, rels, filepath.Join("common", "shared.yaml")) +} diff --git a/pkg/resolver/kustomize/generator_origin_test.go b/pkg/resolver/kustomize/generator_origin_test.go new file mode 100644 index 000000000..f26fcdd7a --- /dev/null +++ b/pkg/resolver/kustomize/generator_origin_test.go @@ -0,0 +1,48 @@ +package kustomize + +import ( + "os" + "path/filepath" + "testing" + + "github.com/DataDog/datadog-iac-scanner/pkg/model" + "github.com/stretchr/testify/require" + krusty "sigs.k8s.io/kustomize/api/krusty" + kustypes "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kyaml/filesys" +) + +func TestOriginFromResource_GeneratorUsesOrgIdNameWithoutHashSuffix(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- name: my-map + literals: + - k=v +`), 0o600)) + require.NoError(t, ensureBuildMetadataIfNeeded(dir)) + + opts := krusty.MakeDefaultOptions() + opts.LoadRestrictions = kustypes.LoadRestrictionsNone + opts.PluginConfig = kustypes.DisabledPluginConfig() + opts.PluginConfig.HelmConfig.Enabled = false + k := krusty.MakeKustomizer(opts) + rm, err := k.Run(filesys.MakeFsOnDisk(), dir) + require.NoError(t, err) + + var found bool + for _, res := range rm.Resources() { + if res.GetGvk().Kind != "ConfigMap" { + continue + } + origin := OriginFromResource(res, dir) + if origin == nil || origin.OriginKind != model.KustomizeOriginGenerator { + continue + } + require.Equal(t, "my-map", origin.ResourceName) + require.NotEqual(t, res.GetName(), origin.ResourceName, "rendered name should include hash suffix while ResourceName stays stable") + found = true + } + require.True(t, found, "expected a generator-origin ConfigMap") +} diff --git a/pkg/resolver/kustomize/konfig_constants_test.go b/pkg/resolver/kustomize/konfig_constants_test.go new file mode 100644 index 000000000..3006e5396 --- /dev/null +++ b/pkg/resolver/kustomize/konfig_constants_test.go @@ -0,0 +1,12 @@ +package kustomize + +import "testing" + +func TestTransformerAnnotationKey_AlignsWithKustomizeAPI(t *testing.T) { + // Must match the annotation key used by sigs.k8s.io/kustomize/api/resource.Resource.GetTransformations + // (sigs.k8s.io/kustomize/api/internal/utils.TransformerAnnotationKey). + const expected = "alpha.config.kubernetes.io/transformations" + if TransformerAnnotationKey != expected { + t.Fatalf("TransformerAnnotationKey = %q, want %q", TransformerAnnotationKey, expected) + } +} diff --git a/pkg/resolver/kustomize/remote_ref.go b/pkg/resolver/kustomize/remote_ref.go new file mode 100644 index 000000000..48fc3adee --- /dev/null +++ b/pkg/resolver/kustomize/remote_ref.go @@ -0,0 +1,69 @@ +package kustomize + +import ( + "path/filepath" + "regexp" + "strings" +) + +var remoteURLSchemes = []string{ + "http://", + "https://", + "ssh://", + "oci://", + "file://", +} + +// scpStyleRE matches "user@host:path" (kustomize SCP-style git remote). +var scpStyleRE = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9-]*@[^/:]+:`) + +// isRemoteKustomizeRef reports whether s would be treated as a remote +// reference by kustomize at render time (sigs.k8s.io/kustomize/api/internal/git, +// v0.21.1) or by go-getter, plus oci:// from #113. +func isRemoteKustomizeRef(s string) bool { + s = strings.TrimSpace(s) + if s == "" || isLocalKustomizePath(s) { + return false + } + return hasRemoteURLScheme(s) || + hasPrefixCaseInsensitive(s, "git::") || + scpStyleRE.MatchString(s) || + isSchemelessGithubHost(s) || + hasGoGetterDoubleSlash(s) || + hasGitSuffixOverDelimiter(s) +} + +func isLocalKustomizePath(s string) bool { + return filepath.IsAbs(s) || + strings.HasPrefix(s, "./") || + strings.HasPrefix(s, "../") || + s == "." || + s == ".." +} + +func hasRemoteURLScheme(s string) bool { + for _, scheme := range remoteURLSchemes { + if hasPrefixCaseInsensitive(s, scheme) { + return true + } + } + return false +} + +// isSchemelessGithubHost mirrors kustomize's isStandardGithubHost. +func isSchemelessGithubHost(s string) bool { + lower := strings.ToLower(s) + return strings.HasPrefix(lower, "github.com/") || strings.HasPrefix(lower, "github.com:") +} + +func hasGoGetterDoubleSlash(s string) bool { + return strings.Contains(s, "//") && strings.Contains(s, ".") +} + +func hasGitSuffixOverDelimiter(s string) bool { + return strings.Contains(s, ".git") && (strings.Contains(s, "://") || strings.Contains(s, "@")) +} + +func hasPrefixCaseInsensitive(s, prefix string) bool { + return len(prefix) <= len(s) && strings.EqualFold(s[:len(prefix)], prefix) +} diff --git a/pkg/resolver/kustomize/transformations_test.go b/pkg/resolver/kustomize/transformations_test.go new file mode 100644 index 000000000..b0714dfdb --- /dev/null +++ b/pkg/resolver/kustomize/transformations_test.go @@ -0,0 +1,80 @@ +package kustomize + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + krusty "sigs.k8s.io/kustomize/api/krusty" + kustypes "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kyaml/filesys" +) + +func TestBuildMetadataIncludesTransformerAnnotations_GetTransformationsPopulated(t *testing.T) { + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + + require.NoError(t, os.WriteFile(filepath.Join(base, "deployment.yaml"), []byte(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx +spec: + replicas: 1 + selector: + matchLabels: + app: nginx + template: + metadata: + labels: + app: nginx + spec: + containers: + - name: nginx + image: nginx +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(base, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- deployment.yaml +`), 0o600)) + + require.NoError(t, os.WriteFile(filepath.Join(overlay, "patch.yaml"), []byte(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx +spec: + replicas: 3 +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +patchesStrategicMerge: +- patch.yaml +`), 0o600)) + + require.NoError(t, ensureBuildMetadataIfNeeded(overlay)) + + opts := krusty.MakeDefaultOptions() + opts.LoadRestrictions = kustypes.LoadRestrictionsNone + opts.PluginConfig = kustypes.DisabledPluginConfig() + opts.PluginConfig.HelmConfig.Enabled = false + k := krusty.MakeKustomizer(opts) + rm, err := k.Run(filesys.MakeFsOnDisk(), overlay) + require.NoError(t, err) + + var anyTrans bool + for _, res := range rm.Resources() { + trans, err := res.GetTransformations() + require.NoError(t, err) + if len(trans) > 0 { + anyTrans = true + break + } + } + require.True(t, anyTrans, "with transformerAnnotations in buildMetadata, GetTransformations should be non-empty for patched resources") +} From 7c068ad8f3edf525c71b5f4e92710703f781286f Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Wed, 29 Apr 2026 13:45:38 +0200 Subject: [PATCH 13/17] Add Kustomize precheck and helm chart inflation prepass with sandbox-confined paths and remote-ref policy. --- pkg/kics/resolver_diagnostic_titles.go | 1 + pkg/resolver/kustomize/helmprepass.go | 433 +++++++++++++++++++++ pkg/resolver/kustomize/helmprepass_test.go | 280 +++++++++++++ pkg/resolver/kustomize/precheck.go | 134 +++++++ pkg/resolver/kustomize/precheck_test.go | 204 ++++++++++ 5 files changed, 1052 insertions(+) create mode 100644 pkg/resolver/kustomize/helmprepass.go create mode 100644 pkg/resolver/kustomize/helmprepass_test.go create mode 100644 pkg/resolver/kustomize/precheck.go create mode 100644 pkg/resolver/kustomize/precheck_test.go diff --git a/pkg/kics/resolver_diagnostic_titles.go b/pkg/kics/resolver_diagnostic_titles.go index 38da99eef..1bf234f3d 100644 --- a/pkg/kics/resolver_diagnostic_titles.go +++ b/pkg/kics/resolver_diagnostic_titles.go @@ -9,6 +9,7 @@ var resolverDiagnosticTitles = map[string]string{ "kustomize-helm-inflation-disabled": "Kustomize Helm inflation disabled", "kustomize-helm-prepass-failed": "Kustomize Helm prepass failed", "kustomize-helm-chart-missing": "Kustomize Helm chart missing", + "kustomize-helm-chart-escape": "Kustomize Helm chart path escapes staged repo", "kustomize-helm-remote-chart-invalid": "Kustomize Helm remote chart invalid", "kustomize-helm-values-invalid": "Kustomize Helm values invalid", "kustomize-helm-render-failed": "Kustomize Helm chart render failed", diff --git a/pkg/resolver/kustomize/helmprepass.go b/pkg/resolver/kustomize/helmprepass.go new file mode 100644 index 000000000..e6e8aaaa9 --- /dev/null +++ b/pkg/resolver/kustomize/helmprepass.go @@ -0,0 +1,433 @@ +package kustomize + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/DataDog/datadog-iac-scanner/pkg/logger" + "github.com/DataDog/datadog-iac-scanner/pkg/model" + "github.com/DataDog/datadog-iac-scanner/pkg/resolver/helm" + "github.com/DataDog/datadog-iac-scanner/pkg/rootfile" + "gopkg.in/yaml.v3" +) + +// ErrMaxStagingBytes is returned when copied helm staging exceeds MaxFetchBytes. +var ErrMaxStagingBytes = errors.New("kustomize staging exceeds max fetch bytes") + +func subtreeDeclaresHelmCharts(kustomRoot, kf string) bool { + var found bool + _ = WalkLocalKustomizations(kustomRoot, func(kustPath string, raw []byte) error { + if filepath.Clean(kustPath) == filepath.Join(kustomRoot, kf) { + return nil + } + var childDoc map[string]interface{} + if err := yaml.Unmarshal(raw, &childDoc); err != nil { + return nil + } + if childHC, _ := childDoc["helmCharts"].([]interface{}); len(childHC) > 0 { + found = true + } + return nil + }) + return found +} + +// PrepareHelmChartsIfNeeded returns the kustomize build dir and fs root; rewrites nested kustomizations with helmCharts. +// If inflation is off, strips helmCharts and emits diagnostics so the rest of the tree can still scan. +func PrepareHelmChartsIfNeeded( + ctx context.Context, + repoRoot, + kustomRoot, + scratchDir string, + allowInflation, + helmIncludeCRDs bool, + maxStagingBytes int64, +) (buildRoot, fsRoot string, diags []model.ResolverDiagnostic, _ error) { + repoRoot = filepath.Clean(repoRoot) + kustomRoot = filepath.Clean(kustomRoot) + kf := kustomizationEntryFile(kustomRoot) + data, err := rootfile.ReadFile(filepath.Join(kustomRoot, kf)) + if err != nil { + return kustomRoot, repoRoot, nil, err + } + var doc map[string]interface{} + if err := yaml.Unmarshal(data, &doc); err != nil { + return kustomRoot, repoRoot, nil, err + } + hc, _ := doc["helmCharts"].([]interface{}) + if len(hc) == 0 && !subtreeDeclaresHelmCharts(kustomRoot, kf) { + return kustomRoot, repoRoot, nil, nil + } + + // Isolate staging per (scratch, kustom root) so same-named overlays do not collide. + sum := sha256.Sum256([]byte(filepath.Clean(scratchDir) + "\x00" + filepath.Clean(kustomRoot))) + stagedRepo := filepath.Join(scratchDir, "helm-prepass", hex.EncodeToString(sum[:16])) + stageRels, err := StageRelPathsForKustomRoot(repoRoot, kustomRoot) + if err != nil { + return kustomRoot, repoRoot, []model.ResolverDiagnostic{{ + FilePath: filepath.Join(kustomRoot, kf), + Message: err.Error(), + QueryID: "kustomize-helm-prepass-failed", + Line: 1, + }}, nil + } + if err := CopyRepoRelativeFilesNoSymlinks(stagedRepo, repoRoot, stageRels); err != nil { + return kustomRoot, repoRoot, []model.ResolverDiagnostic{{ + FilePath: filepath.Join(kustomRoot, kf), + Message: err.Error(), + QueryID: "kustomize-helm-prepass-failed", + Line: 1, + }}, nil + } + relFromRepo, err := filepath.Rel(repoRoot, kustomRoot) + if err != nil || relFromRepo == relDotDot || strings.HasPrefix(relFromRepo, parentDirPrefix) { + return kustomRoot, repoRoot, []model.ResolverDiagnostic{{ + FilePath: filepath.Join(kustomRoot, kf), + Message: "kustomization root is outside configured repo root", + QueryID: "kustomize-helm-prepass-failed", + Line: 1, + }}, nil + } + staging := filepath.Join(stagedRepo, relFromRepo) + if maxStagingBytes > 0 { + sz, err := DirTotalSize(stagedRepo) + if err == nil && sz > maxStagingBytes { + return kustomRoot, stagedRepo, []model.ResolverDiagnostic{{ + FilePath: filepath.Join(kustomRoot, kf), + Message: "helm prepass copy exceeds configured Kustomize max fetch bytes", + QueryID: "kustomize-max-fetch-exceeded", + Line: 1, + }}, ErrMaxStagingBytes + } + } + + configHome := filepath.Join(scratchDir, "helm-config") + repoConfig := filepath.Join(configHome, "repositories.yaml") + repoCache := filepath.Join(configHome, ".cache") + registryConfig := filepath.Join(configHome, "registry", "config.json") + _ = os.MkdirAll(filepath.Dir(repoConfig), dirPermCopyTree) + _ = os.MkdirAll(repoCache, dirPermCopyTree) + _ = os.MkdirAll(filepath.Dir(registryConfig), dirPermCopyTree) + + if err := WalkLocalKustomizations(staging, func(kustPath string, raw []byte) error { + var err error + diags, err = rewriteHelmChartsInKustomization( + ctx, kustPath, raw, stagedRepo, + helmIncludeCRDs, allowInflation, + repoConfig, repoCache, registryConfig, diags, + ) + return err + }); err != nil { + return kustomRoot, stagedRepo, diags, err + } + return staging, stagedRepo, diags, nil +} + +func renderHelmChartEntries( + ctx context.Context, + hc []interface{}, + kustPath, kustDir, stagedRepo, genDir, chartHome string, + helmIncludeCRDs bool, + repoConfig, repoCache, registryConfig string, + diags []model.ResolverDiagnostic, +) ([]string, []model.ResolverDiagnostic) { + var newResources []string + for i, item := range hc { + entry, ok := item.(map[string]interface{}) + if !ok { + continue + } + name, _ := entry["name"].(string) + if name == "" { + continue + } + repo, _ := entry["repo"].(string) + version, _ := entry["version"].(string) + + chartPath, chartDiags, ok := resolveHelmChartPath(ctx, kustPath, stagedRepo, kustDir, chartHome, name, repo) + diags = append(diags, chartDiags...) + if !ok { + continue + } + + valuesFiles, err := helmValueFilesForEntry(stagedRepo, kustDir, entry) + if err != nil { + diags = append(diags, model.ResolverDiagnostic{ + FilePath: kustPath, + Message: err.Error(), + QueryID: "kustomize-helm-values-invalid", + Line: 1, + }) + continue + } + + releaseName, _ := entry["releaseName"].(string) + nameTemplate, _ := entry["nameTemplate"].(string) + namespace, _ := entry["namespace"].(string) + valuesInline := mapStringAny(entry["valuesInline"]) + valuesMerge, _ := entry["valuesMerge"].(string) + apiVersions := stringSlice(entry["apiVersions"]) + kubeVersion, _ := entry["kubeVersion"].(string) + devel, _ := entry["devel"].(bool) + + includeCRDs := helmIncludeCRDs + if v, ok := entry["includeCRDs"].(bool); ok { + includeCRDs = v + } + skipHooks := false + if v, ok := entry["skipHooks"].(bool); ok && v { + skipHooks = true + } + skipTests := false + if v, ok := entry["skipTests"].(bool); ok && v { + skipTests = true + } + + rc, err := helm.RenderChart(ctx, &helm.RenderOptions{ + ChartPath: chartPath, + ChartRepo: strings.TrimSpace(repo), + ReleaseName: strings.TrimSpace(releaseName), + NameTemplate: strings.TrimSpace(nameTemplate), + Namespace: strings.TrimSpace(namespace), + ValuesFiles: valuesFiles, + ValuesInline: valuesInline, + ValuesMerge: strings.TrimSpace(valuesMerge), + IncludeCRDs: includeCRDs, + SkipHooks: skipHooks, + SkipTests: skipTests, + APIVersions: apiVersions, + KubeVersion: strings.TrimSpace(kubeVersion), + Version: strings.TrimSpace(version), + Devel: devel, + RepositoryConfig: repoConfig, + RepositoryCache: repoCache, + RegistryConfig: registryConfig, + }) + if err != nil { + diags = append(diags, model.ResolverDiagnostic{ + FilePath: kustPath, + Message: err.Error(), + QueryID: "kustomize-helm-render-failed", + Line: 1, + }) + continue + } + outPath := filepath.Join(genDir, filepath.Base(name)+"-"+strconv.Itoa(i)+".yaml") + var blobs [][]byte + for _, res := range rc.Resources { + blobs = append(blobs, res.Content) + } + if err := os.WriteFile(outPath, joinYAML(blobs), filePermCopyTree); err != nil { + diags = append(diags, model.ResolverDiagnostic{ + FilePath: outPath, + Message: err.Error(), + QueryID: "kustomize-helm-write-failed", + Line: 1, + }) + continue + } + rel, _ := filepath.Rel(kustDir, outPath) + newResources = append(newResources, rel) + } + return newResources, diags +} + +func rewriteHelmChartsInKustomization( + ctx context.Context, + kustPath string, + raw []byte, + stagedRepo string, + helmIncludeCRDs, allowInflation bool, + repoConfig, repoCache, registryConfig string, + diags []model.ResolverDiagnostic, +) ([]model.ResolverDiagnostic, error) { + var doc map[string]interface{} + if err := yaml.Unmarshal(raw, &doc); err != nil { + return diags, err + } + hc, _ := doc["helmCharts"].([]interface{}) + if len(hc) == 0 { + return diags, nil + } + kustDir := filepath.Dir(kustPath) + if !allowInflation { + delete(doc, "helmCharts") + delete(doc, "helmGlobals") + outK, err := yaml.Marshal(doc) + if err != nil { + return diags, err + } + if err := os.WriteFile(kustPath, outK, filePermCopyTree); err != nil { + return diags, err + } + return append(diags, model.ResolverDiagnostic{ + FilePath: kustPath, + Message: "kustomization declares helmCharts but Kustomize Helm inflation is disabled", + QueryID: "kustomize-helm-inflation-disabled", + Line: 1, + }), nil + } + + chartHome := defaultHelmChartHome + if g, ok := doc["helmGlobals"].(map[string]interface{}); ok { + if ch, ok := g["chartHome"].(string); ok && ch != "" { + chartHome = ch + } + } + genDir := filepath.Join(kustDir, ".iac-scanner-helm-out") + _ = os.MkdirAll(genDir, dirPermCopyTree) + + newResources, diags := renderHelmChartEntries( + ctx, hc, kustPath, kustDir, stagedRepo, genDir, chartHome, + helmIncludeCRDs, repoConfig, repoCache, registryConfig, diags, + ) + + delete(doc, "helmCharts") + delete(doc, "helmGlobals") + existing, _ := doc["resources"].([]interface{}) + resList := append([]interface{}(nil), existing...) + for _, r := range newResources { + resList = append(resList, r) + } + doc["resources"] = resList + outK, err := yaml.Marshal(doc) + if err != nil { + return diags, err + } + if err := os.WriteFile(kustPath, outK, filePermCopyTree); err != nil { + return diags, err + } + return diags, nil +} + +func helmValueFilesForEntry(repoRoot, staging string, entry map[string]interface{}) ([]string, error) { + var files []string + if vf, ok := entry["valuesFile"].(string); ok && vf != "" { + p, err := stagedHelmValuePath(repoRoot, staging, vf) + if err != nil { + return nil, err + } + files = append(files, p) + } + if av, ok := entry["additionalValuesFiles"].([]interface{}); ok { + for _, x := range av { + if s, ok := x.(string); ok && s != "" { + p, err := stagedHelmValuePath(repoRoot, staging, s) + if err != nil { + return nil, err + } + files = append(files, p) + } + } + } + // Inline values go through RenderOptions.ValuesInline in helm.RenderChart, not ValueFiles here. + return files, validateValuesMerge(entry) +} + +func stagedHelmValuePath(stagedRepoRoot, staging, rel string) (string, error) { + stagedRepoRoot = filepath.Clean(stagedRepoRoot) + staging = filepath.Clean(staging) + p := filepath.Clean(filepath.Join(staging, rel)) + if !isUnderRoot(p, stagedRepoRoot) { + return "", fmt.Errorf("helm values file %q escapes the staged repo root", rel) + } + return p, nil +} + +// stagedHelmChartPath resolves a local helmCharts entry to a path under +// stagedRepoRoot, rejecting "../" escapes via chartHome or name. +func stagedHelmChartPath(stagedRepoRoot, kustDir, chartHome, name string) (string, error) { + stagedRepoRoot = filepath.Clean(stagedRepoRoot) + kustDir = filepath.Clean(kustDir) + p := filepath.Clean(filepath.Join(kustDir, chartHome, name)) + if !isUnderRoot(p, stagedRepoRoot) { + return "", fmt.Errorf("helm chart %q (chartHome %q) escapes the staged repo root", name, chartHome) + } + return p, nil +} + +// resolveHelmChartPath returns the chart locator for a helmCharts entry. For +// remote charts (repo != "") it is the chart name; for local charts it is the +// staged path. ok is false when the entry must be skipped. +func resolveHelmChartPath( + ctx context.Context, + kustPath, stagedRepo, kustDir, chartHome, name, repo string, +) (string, []model.ResolverDiagnostic, bool) { + if strings.TrimSpace(repo) != "" { + return name, nil, true + } + contextLogger := logger.FromContext(ctx) + localPath, err := stagedHelmChartPath(stagedRepo, kustDir, chartHome, name) + if err != nil { + return "", []model.ResolverDiagnostic{{ + FilePath: kustPath, + Message: err.Error(), + QueryID: "kustomize-helm-chart-escape", + Line: 1, + }}, false + } + if _, err := os.Stat(localPath); err != nil { + contextLogger.Warn().Msgf("kustomize helm chart not found at %s", localPath) + return "", []model.ResolverDiagnostic{{ + FilePath: kustPath, + Message: "helm chart not found: " + localPath, + QueryID: "kustomize-helm-chart-missing", + Line: 1, + }}, false + } + return localPath, nil, true +} + +func validateValuesMerge(entry map[string]interface{}) error { + mode, _ := entry["valuesMerge"].(string) + mode = strings.TrimSpace(mode) + switch mode { + case "", "merge", "override", "replace": + return nil + default: + return fmt.Errorf("invalid valuesMerge %q (expected merge, override, or replace)", mode) + } +} + +func mapStringAny(v interface{}) map[string]interface{} { + m, ok := v.(map[string]interface{}) + if !ok { + return nil + } + return m +} + +func stringSlice(v interface{}) []string { + list, ok := v.([]interface{}) + if !ok { + return nil + } + out := make([]string, 0, len(list)) + for _, item := range list { + if s, ok := item.(string); ok && strings.TrimSpace(s) != "" { + out = append(out, strings.TrimSpace(s)) + } + } + return out +} + +func joinYAML(parts [][]byte) []byte { + var b []byte + for i, p := range parts { + if i > 0 { + b = append(b, "---\n"...) + } + b = append(b, p...) + if len(p) > 0 && p[len(p)-1] != '\n' { + b = append(b, '\n') + } + } + return b +} diff --git a/pkg/resolver/kustomize/helmprepass_test.go b/pkg/resolver/kustomize/helmprepass_test.go new file mode 100644 index 000000000..0067b157d --- /dev/null +++ b/pkg/resolver/kustomize/helmprepass_test.go @@ -0,0 +1,280 @@ +package kustomize + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPrepareHelmChartsIfNeeded_ErrMaxStagingBytes(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + scratch := t.TempDir() + charts := filepath.Join(root, "charts", "mini") + require.NoError(t, os.MkdirAll(filepath.Join(charts, "templates"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "Chart.yaml"), []byte("apiVersion: v2\nname: mini\nversion: 0.1.0\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "values.yaml"), []byte("{}"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "templates", "cm.yaml"), []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: x\ndata: {}\n"), 0o600)) + + kust := `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +helmCharts: +- name: mini + releaseName: r +` + require.NoError(t, os.WriteFile(filepath.Join(root, "kustomization.yaml"), []byte(kust), 0o600)) + + _, _, diags, err := PrepareHelmChartsIfNeeded(ctx, root, root, scratch, true, true, 1) + require.ErrorIs(t, err, ErrMaxStagingBytes) + require.NotEmpty(t, diags) +} + +func TestPrepareHelmChartsIfNeeded_InvalidValuesMerge(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + scratch := t.TempDir() + charts := filepath.Join(root, "charts", "mini") + require.NoError(t, os.MkdirAll(filepath.Join(charts, "templates"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "Chart.yaml"), []byte("apiVersion: v2\nname: mini\nversion: 0.1.0\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "values.yaml"), []byte("{}\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "templates", "cm.yaml"), []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: x\n"), 0o600)) + kust := `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +helmCharts: +- name: mini + valuesMerge: banana +` + require.NoError(t, os.WriteFile(filepath.Join(root, "kustomization.yaml"), []byte(kust), 0o600)) + _, _, diags, err := PrepareHelmChartsIfNeeded(ctx, root, root, scratch, true, true, 0) + require.NoError(t, err) + require.NotEmpty(t, diags) + require.Equal(t, "kustomize-helm-values-invalid", diags[0].QueryID) +} + +func TestPrepareHelmChartsIfNeeded_StagesSiblingBaseRefs(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + charts := filepath.Join(overlay, "charts", "mini") + require.NoError(t, os.MkdirAll(filepath.Join(charts, "templates"), 0o700)) + require.NoError(t, os.MkdirAll(base, 0o700)) + + require.NoError(t, os.WriteFile(filepath.Join(charts, "Chart.yaml"), []byte("apiVersion: v2\nname: mini\nversion: 0.1.0\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "values.yaml"), []byte("{}\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "templates", "cm.yaml"), []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: from-helm\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(base, "cm.yaml"), []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: from-base\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(base, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- cm.yaml +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +helmCharts: +- name: mini + releaseName: rel +`), 0o600)) + + buildRoot, fsRoot, diags, err := PrepareHelmChartsIfNeeded(ctx, repo, overlay, t.TempDir(), true, true, 0) + require.NoError(t, err) + require.Empty(t, diags) + require.NotEqual(t, overlay, buildRoot) + require.Equal(t, filepath.Dir(buildRoot), fsRoot) + + raw, err := os.ReadFile(filepath.Join(buildRoot, "kustomization.yaml")) + require.NoError(t, err) + txt := string(raw) + require.Contains(t, txt, "../base") + require.NotContains(t, txt, "helmCharts:") + + stagedBaseFile := filepath.Join(fsRoot, "base", "cm.yaml") + _, err = os.Stat(stagedBaseFile) + require.NoError(t, err) + + stagedContent, err := os.ReadFile(filepath.Join(buildRoot, "kustomization.yaml")) + require.NoError(t, err) + require.True(t, strings.Contains(string(stagedContent), "../base") || strings.Contains(string(stagedContent), "base")) +} + +func TestPrepareHelmChartsIfNeeded_StagesCustomValuesFiles(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + overlay := filepath.Join(repo, "overlay") + charts := filepath.Join(overlay, "charts", "mini") + require.NoError(t, os.MkdirAll(filepath.Join(charts, "templates"), 0o700)) + + require.NoError(t, os.WriteFile(filepath.Join(charts, "Chart.yaml"), []byte("apiVersion: v2\nname: mini\nversion: 0.1.0\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "values.yaml"), []byte("service:\n port: 80\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "templates", "svc.yaml"), []byte("apiVersion: v1\nkind: Service\nmetadata:\n name: svc\nspec:\n ports:\n - port: {{ .Values.service.port }}\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "custom-values.yaml"), []byte("service:\n port: 9090\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +helmCharts: +- name: mini + releaseName: rel + valuesFile: custom-values.yaml +`), 0o600)) + + buildRoot, fsRoot, diags, err := PrepareHelmChartsIfNeeded(ctx, repo, overlay, t.TempDir(), true, true, 0) + require.NoError(t, err) + require.Empty(t, diags) + require.Equal(t, filepath.Dir(buildRoot), fsRoot) + _, err = os.Stat(filepath.Join(buildRoot, "custom-values.yaml")) + require.NoError(t, err) +} + +func TestPrepareHelmChartsIfNeeded_RewritesNestedBaseHelmCharts(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + charts := filepath.Join(base, "charts", "mini") + require.NoError(t, os.MkdirAll(filepath.Join(charts, "templates"), 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + + require.NoError(t, os.WriteFile(filepath.Join(charts, "Chart.yaml"), []byte("apiVersion: v2\nname: mini\nversion: 0.1.0\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "values.yaml"), []byte("{}\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "templates", "cm.yaml"), []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: nested\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(base, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +helmCharts: +- name: mini + releaseName: rel +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +`), 0o600)) + + buildRoot, _, diags, err := PrepareHelmChartsIfNeeded(ctx, repo, overlay, t.TempDir(), true, true, 0) + require.NoError(t, err) + require.Empty(t, diags) + + stagedBaseKust := filepath.Join(filepath.Dir(buildRoot), "base", "kustomization.yaml") + raw, err := os.ReadFile(stagedBaseKust) + require.NoError(t, err) + require.NotContains(t, string(raw), "helmCharts:") + require.Contains(t, string(raw), ".iac-scanner-helm-out") +} + +func TestPrepareHelmChartsIfNeeded_StripsHelmChartsWhenInflationDisabled(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + charts := filepath.Join(repo, "charts", "mini") + require.NoError(t, os.MkdirAll(filepath.Join(charts, "templates"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "Chart.yaml"), []byte("apiVersion: v2\nname: mini\nversion: 0.1.0\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "values.yaml"), []byte("{}\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(charts, "templates", "cm.yaml"), []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: disabled\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(repo, "local.yaml"), []byte("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: keep-me\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(repo, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- local.yaml +helmCharts: +- name: mini + releaseName: rel +`), 0o600)) + + buildRoot, _, diags, err := PrepareHelmChartsIfNeeded(ctx, repo, repo, t.TempDir(), false, true, 0) + require.NoError(t, err) + require.NotEmpty(t, diags) + require.Equal(t, "kustomize-helm-inflation-disabled", diags[0].QueryID) + + raw, err := os.ReadFile(filepath.Join(buildRoot, "kustomization.yaml")) + require.NoError(t, err) + require.NotContains(t, string(raw), "helmCharts:") + require.Contains(t, string(raw), "local.yaml") +} + +func TestPrepareHelmChartsIfNeeded_DoesNotPreRejectRemoteRepoWithoutVersion(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(repo, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +helmCharts: +- name: mini + repo: https://example.com/charts + releaseName: rel +`), 0o600)) + + _, _, diags, err := PrepareHelmChartsIfNeeded(ctx, repo, repo, t.TempDir(), true, true, 0) + require.NoError(t, err) + for _, d := range diags { + require.NotEqual(t, "kustomize-helm-remote-chart-invalid", d.QueryID) + } +} + +func TestHelmValueFilesForEntry_RejectsEscape(t *testing.T) { + root := t.TempDir() + staging := filepath.Join(root, "overlay") + entry := map[string]interface{}{ + "valuesFile": "../../secrets.yaml", + } + _, err := helmValueFilesForEntry(root, staging, entry) + require.Error(t, err) + require.Contains(t, err.Error(), "escapes the staged repo root") +} + +func TestStagedHelmChartPath_RejectsEscape(t *testing.T) { + stagedRepo := t.TempDir() + kustDir := filepath.Join(stagedRepo, "overlay") + + cases := []struct { + name string + chartHome string + chart string + }{ + {"chartHome traversal", "../../../etc", "passwd"}, + {"chart name traversal", "charts", "../../../etc/passwd"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := stagedHelmChartPath(stagedRepo, kustDir, tc.chartHome, tc.chart) + require.Error(t, err) + require.Contains(t, err.Error(), "escapes the staged repo root") + }) + } +} + +func TestStagedHelmChartPath_AcceptsInRoot(t *testing.T) { + stagedRepo := t.TempDir() + kustDir := filepath.Join(stagedRepo, "overlay") + require.NoError(t, os.MkdirAll(filepath.Join(kustDir, "charts", "mini"), 0o700)) + + got, err := stagedHelmChartPath(stagedRepo, kustDir, "charts", "mini") + require.NoError(t, err) + require.Equal(t, filepath.Join(kustDir, "charts", "mini"), got) +} + +func TestPrepareHelmChartsIfNeeded_RejectsLocalChartEscape(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(repo, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +helmGlobals: + chartHome: ../../../etc +helmCharts: +- name: passwd + releaseName: rel +`), 0o600)) + + _, _, diags, err := PrepareHelmChartsIfNeeded(ctx, repo, repo, t.TempDir(), true, true, 0) + require.NoError(t, err) + var sawEscape bool + for _, d := range diags { + if d.QueryID == "kustomize-helm-chart-escape" { + sawEscape = true + require.Contains(t, d.Message, "escapes the staged repo root") + } + require.NotEqual(t, "kustomize-helm-render-failed", d.QueryID) + } + require.True(t, sawEscape, "%+v", diags) +} diff --git a/pkg/resolver/kustomize/precheck.go b/pkg/resolver/kustomize/precheck.go new file mode 100644 index 000000000..02f87f544 --- /dev/null +++ b/pkg/resolver/kustomize/precheck.go @@ -0,0 +1,134 @@ +package kustomize + +import ( + "os" + "path/filepath" + + "gopkg.in/yaml.v3" +) + +func appendRemoteRefsFromResourceItems(v interface{}, add func(string)) { + t, ok := v.([]interface{}) + if !ok { + return + } + for _, item := range t { + switch x := item.(type) { + case string: + add(x) + case map[string]interface{}: + for _, subk := range []string{"repo", "url", "path"} { + if s, ok := x[subk].(string); ok { + add(s) + } + } + } + } +} + +func appendRemoteRefsFromPatchLikeItems(v interface{}, add func(string)) { + t, ok := v.([]interface{}) + if !ok { + return + } + for _, item := range t { + switch x := item.(type) { + case string: + add(x) + case map[string]interface{}: + if s, ok := x["path"].(string); ok { + add(s) + } + if s, ok := x["repo"].(string); ok { + add(s) + } + if s, ok := x["url"].(string); ok { + add(s) + } + } + } +} + +func appendRemoteRefsFromOpenAPI(openapi map[string]interface{}, add func(string)) { + if s, ok := openapi["path"].(string); ok { + add(s) + } + if s, ok := openapi["repo"].(string); ok { + add(s) + } + if s, ok := openapi["url"].(string); ok { + add(s) + } +} + +// CollectRemoteRefsFromKustomization lists remote-like refs in a kustomization +// document. helmCharts are intentionally excluded; the Helm prepass owns +// chart repos and gracefully handles the inflation-disabled path. +func CollectRemoteRefsFromKustomization(data []byte) ([]string, error) { + var doc map[string]interface{} + if err := yaml.Unmarshal(data, &doc); err != nil { + return nil, err + } + var out []string + add := func(s string) { + if isRemoteKustomizeRef(s) { + out = append(out, s) + } + } + for _, key := range []string{"resources", "components", "bases"} { + if v, ok := doc[key]; ok { + appendRemoteRefsFromResourceItems(v, add) + } + } + for _, key := range []string{"patches", "generators", "transformers", "validators", "patchesJson6902"} { + if v, ok := doc[key]; ok { + appendRemoteRefsFromPatchLikeItems(v, add) + } + } + if openapi, ok := doc["openapi"].(map[string]interface{}); ok { + appendRemoteRefsFromOpenAPI(openapi, add) + } + return out, nil +} + +// DetectKRMInlineFunctions is true when generators/transformers/validators use inline KRM objects (not executed here). +func DetectKRMInlineFunctions(doc map[string]interface{}) bool { + for _, key := range []string{"generators", "transformers", "validators"} { + v, ok := doc[key] + if !ok { + continue + } + list, ok := v.([]interface{}) + if !ok { + continue + } + for _, item := range list { + m, ok := item.(map[string]interface{}) + if !ok { + continue + } + if _, hasPath := m["path"]; hasPath { + continue + } + if m["apiVersion"] != nil && m["kind"] != nil { + return true + } + } + } + return false +} + +// DirTotalSize returns the sum of regular file sizes under root (best-effort). +func DirTotalSize(root string) (int64, error) { + var n int64 + err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.Mode().IsRegular() { + n += info.Size() + } + return nil + }) + return n, err +} diff --git a/pkg/resolver/kustomize/precheck_test.go b/pkg/resolver/kustomize/precheck_test.go new file mode 100644 index 000000000..0f0458b60 --- /dev/null +++ b/pkg/resolver/kustomize/precheck_test.go @@ -0,0 +1,204 @@ +package kustomize + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +func TestCollectRemoteRefsFromKustomization(t *testing.T) { + raw := `resources: +- ./local +- https://example.com/base +- git::https://github.com/org/repo.git//kustomize?ref=main +- github.com/org/another-repo//deploy?ref=main +- github.com/org/plain-repo//base +- github.com/Liujingfang1/mysql?ref=test +bases: +- git@github.com:org/repo.git//deploy?ref=main +components: +- ssh://git@example.com/acme/comp.git +patches: +- path: https://example.com/patch.yaml +transformers: +- path: github.com/org/transformers//labeler +generators: +- path: git::https://github.com/org/plugin.git//gen +helmCharts: +- name: foo + repo: oci://registry.example.com/charts/foo +` + got, err := CollectRemoteRefsFromKustomization([]byte(raw)) + require.NoError(t, err) + require.ElementsMatch(t, []string{ + "https://example.com/base", + "git::https://github.com/org/repo.git//kustomize?ref=main", + "github.com/org/another-repo//deploy?ref=main", + "github.com/org/plain-repo//base", + "github.com/Liujingfang1/mysql?ref=test", + "git@github.com:org/repo.git//deploy?ref=main", + "ssh://git@example.com/acme/comp.git", + "https://example.com/patch.yaml", + "github.com/org/transformers//labeler", + "git::https://github.com/org/plugin.git//gen", + }, got) +} + +func TestIsRemoteKustomizeRef(t *testing.T) { + cases := []struct { + in string + want bool + }{ + {"", false}, + {".", false}, + {"./local", false}, + {"../base", false}, + {"base", false}, + {"my.dir/file.yaml", false}, + {"/abs/path", false}, + {"server/foo/bar", false}, + {"my-base?ref=foo", false}, + + {"https://example.com/base", true}, + {"HTTPS://EXAMPLE.com/base", true}, + {"http://example.com/base", true}, + {"ssh://git@example.com/acme/comp.git", true}, + {"oci://registry.example.com/charts/foo", true}, + {"file:///etc/passwd", true}, + {"git::https://github.com/org/plugin.git//gen", true}, + {"git@github.com:org/repo.git//deploy?ref=main", true}, + {"git@gitlab2.example.com:infra/repo.git?ref=v1", true}, + {"org-12345@github.com:foo/bar", true}, + {"github.com/Liujingfang1/mysql?ref=test", true}, + {"GitHub.com/foo/bar", true}, + {"github.com/foo/bar//sub?ref=v1", true}, + {"example.org/path/to/repo//examples/multibases/dev", true}, + {"https://example.com/path/to/repo.git/sub", true}, + } + for _, tc := range cases { + t.Run(tc.in, func(t *testing.T) { + require.Equal(t, tc.want, isRemoteKustomizeRef(tc.in), "input: %q", tc.in) + }) + } +} + +func TestCollectRemoteRefsFromKustomization_HelmChartsAreOwnedByPrepass(t *testing.T) { + raw := `helmCharts: +- name: chart-a + repo: oci://registry.example.com/charts/foo +- name: chart-b + repo: https://charts.example.com +` + got, err := CollectRemoteRefsFromKustomization([]byte(raw)) + require.NoError(t, err) + require.Empty(t, got) +} + +func TestDetectKRMInlineFunctions(t *testing.T) { + var doc map[string]interface{} + require.NoError(t, yaml.Unmarshal([]byte(`generators: +- apiVersion: builtin + kind: ConfigMapGenerator + name: x +`), &doc)) + require.True(t, DetectKRMInlineFunctions(doc)) + + require.NoError(t, yaml.Unmarshal([]byte(`generators: +- path: ./plugin.yaml +`), &doc)) + require.False(t, DetectKRMInlineFunctions(doc)) +} + +func TestTransitiveLocalPaths_ReplacementsOnlyCollectsPathFields(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +replacements: +- source: + kind: ConfigMap + name: app-config + fieldPath: data.message + targets: + - select: + kind: Deployment + name: app + options: + delimiter: "." + index: 1 + fieldPaths: + - spec.template.spec.containers.0.image +- path: repl.yaml +`), 0o600)) + got, err := TransitiveLocalPaths(dir) + require.NoError(t, err) + require.Equal(t, []string{ + filepath.Join(dir, "kustomization.yaml"), + filepath.Join(dir, "repl.yaml"), + }, got) +} + +func TestTransitiveLocalPaths_IncludesUpwardLocalRefsAndEntryFile(t *testing.T) { + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +patches: +- path: ../base/patch.yaml +`), 0o600)) + + got, err := TransitiveLocalPaths(overlay) + require.NoError(t, err) + require.Equal(t, []string{ + filepath.Join(overlay, "kustomization.yaml"), + filepath.Join(base), + filepath.Join(base, "patch.yaml"), + }, got) +} + +func TestTransitiveLocalPaths_RecursesIntoNestedLocalKustomizations(t *testing.T) { + repo := t.TempDir() + common := filepath.Join(repo, "common") + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(common, 0o700)) + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(base, "Kustomization"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../common/shared.yaml +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +`), 0o600)) + + got, err := TransitiveLocalPaths(overlay) + require.NoError(t, err) + require.Contains(t, got, filepath.Join(overlay, "kustomization.yml")) + require.Contains(t, got, filepath.Join(base, "Kustomization")) + require.Contains(t, got, filepath.Join(common, "shared.yaml")) +} + +func TestTransitiveRelativeLocalPaths_IgnoresAbsoluteRefs(t *testing.T) { + repo := t.TempDir() + external := t.TempDir() + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(overlay, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yaml"), []byte("apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\nresources:\n- ../shared\n- "+filepath.Join(external, "base")+"\n"), 0o600)) + + got, err := TransitiveRelativeLocalPaths(overlay) + require.NoError(t, err) + require.Contains(t, got, overlay) + require.Contains(t, got, filepath.Join(repo, "shared")) + require.NotContains(t, got, filepath.Join(external, "base")) +} From 0a92c0ffa79f50126a68067d94ce3f3cda722e36 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Wed, 29 Apr 2026 13:45:48 +0200 Subject: [PATCH 14/17] Wire the Kustomize resolver into the scanner via a subprocess render helper, repo/scratch-bounded metadata reads, and CLI re-exec. --- cmd/scanner/internal.go | 26 + cmd/scanner/main.go | 3 + pkg/resolver/kustomize/helper.go | 93 ++++ pkg/resolver/kustomize/helper_main_test.go | 13 + pkg/resolver/kustomize/resolver.go | 424 ++++++++++++++ pkg/resolver/kustomize/resolver_phases.go | 273 +++++++++ pkg/resolver/kustomize/resolver_test.go | 610 +++++++++++++++++++++ 7 files changed, 1442 insertions(+) create mode 100644 cmd/scanner/internal.go create mode 100644 pkg/resolver/kustomize/helper.go create mode 100644 pkg/resolver/kustomize/helper_main_test.go create mode 100644 pkg/resolver/kustomize/resolver.go create mode 100644 pkg/resolver/kustomize/resolver_phases.go create mode 100644 pkg/resolver/kustomize/resolver_test.go diff --git a/cmd/scanner/internal.go b/cmd/scanner/internal.go new file mode 100644 index 000000000..a6dc13c5d --- /dev/null +++ b/cmd/scanner/internal.go @@ -0,0 +1,26 @@ +package main + +import ( + "context" + + "github.com/DataDog/datadog-iac-scanner/pkg/resolver/kustomize" + cli "github.com/urfave/cli/v3" +) + +// internalAction holds hidden subcommands the scanner re-execs (e.g. kustomize +// render in a child the parent can kill on timeout). Hidden from --help. +var internalAction = &cli.Command{ + Name: "internal", + Hidden: true, + Usage: "internal helpers used by the scanner to re-exec itself; not for direct use", + Commands: []*cli.Command{ + { + Name: "kustomize-render", + Hidden: true, + Usage: "run a single kustomize build; reads a JSON request from stdin and writes a JSON response to stdout", + Action: func(ctx context.Context, c *cli.Command) error { + return kustomize.RunHelperFromStdin() + }, + }, + }, +} diff --git a/cmd/scanner/main.go b/cmd/scanner/main.go index ceeade99c..59ece2b56 100644 --- a/cmd/scanner/main.go +++ b/cmd/scanner/main.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/DataDog/datadog-iac-scanner/pkg/resolver/kustomize" "github.com/rs/zerolog" "github.com/rs/zerolog/log" cli "github.com/urfave/cli/v3" @@ -22,6 +23,7 @@ const defaultFailCode = 126 const gcPercent = 50 func main() { + kustomize.MaybeRunAsKustomizeRenderHelper() if _, ok := os.LookupEnv("GOGC"); !ok { debug.SetGCPercent(gcPercent) } @@ -32,6 +34,7 @@ func main() { scanAction, listPlatformsAction, listQueriesAction, + internalAction, }, Flags: []cli.Flag{ &cli.StringFlag{ diff --git a/pkg/resolver/kustomize/helper.go b/pkg/resolver/kustomize/helper.go new file mode 100644 index 000000000..e9ae224be --- /dev/null +++ b/pkg/resolver/kustomize/helper.go @@ -0,0 +1,93 @@ +package kustomize + +import ( + "encoding/json" + "fmt" + "io" + "os" + "strconv" + "time" + + "github.com/DataDog/datadog-iac-scanner/pkg/resolver/sandbox" + krusty "sigs.k8s.io/kustomize/api/krusty" + kustypes "sigs.k8s.io/kustomize/api/types" +) + +// helperEnvVar: set by renderWithTimeout when re-execing a test binary (no CLI +// binary). Production uses `internal kustomize-render` instead. +const helperEnvVar = "GO_WANT_KUSTOMIZE_RENDER_HELPER" + +// helperSleepEnvVar: test-only delay before stdin handling (timeout regression). +const helperSleepEnvVar = "KUSTOMIZE_HELPER_SLEEP_MS" + +// MaybeRunAsKustomizeRenderHelper exits early when helperEnvVar=1 (test subprocess +// re-exec); call from main before the CLI runs. No-op in normal/production runs. +func MaybeRunAsKustomizeRenderHelper() { + if os.Getenv(helperEnvVar) != "1" { + return + } + if sleepMs := os.Getenv(helperSleepEnvVar); sleepMs != "" { + if n, err := strconv.Atoi(sleepMs); err == nil && n > 0 { + time.Sleep(time.Duration(n) * time.Millisecond) + } + } + _ = RunHelperFromStdin() + os.Exit(0) +} + +// helperRequest is the JSON body on stdin for one kustomize build. +type helperRequest struct { + BuildRoot string `json:"build_root"` + RunFSRoot string `json:"run_fs_root"` + StrictLoad bool `json:"strict_load"` +} + +// buildResult is JSON on stdout; render errors use Err (timeouts/crashes are parent-side). +type buildResult struct { + YAML string `json:"yaml"` + Err string `json:"err,omitempty"` +} + +// RunHelperFromStdin is the `internal kustomize-render` entrypoint: stdin JSON +// request, stdout JSON result; render failures are encoded in buildResult.Err. +func RunHelperFromStdin() error { + body, err := io.ReadAll(os.Stdin) + if err != nil { + return writeHelperResult(buildResult{Err: fmt.Sprintf("read stdin: %v", err)}) + } + var req helperRequest + if err := json.Unmarshal(body, &req); err != nil { + return writeHelperResult(buildResult{Err: fmt.Sprintf("decode helper request: %v", err)}) + } + return writeHelperResult(helperRender(req.BuildRoot, req.RunFSRoot, req.StrictLoad)) +} + +func writeHelperResult(r buildResult) error { + return json.NewEncoder(os.Stdout).Encode(r) +} + +// helperRender runs krusty in-process (shared by CLI subcommand and test re-exec). +func helperRender(buildRoot, runFSRoot string, strictLoad bool) buildResult { + opts := krusty.MakeDefaultOptions() + if strictLoad { + opts.LoadRestrictions = kustypes.LoadRestrictionsRootOnly + } else { + opts.LoadRestrictions = kustypes.LoadRestrictionsNone + } + opts.PluginConfig = kustypes.DisabledPluginConfig() + opts.PluginConfig.HelmConfig.Enabled = false + k := krusty.MakeKustomizer(opts) + runFS, err := sandbox.NewBoundedFS(runFSRoot) + if err != nil { + return buildResult{Err: err.Error()} + } + rm, err := k.Run(runFS, buildRoot) + if err != nil { + return buildResult{Err: err.Error()} + } + yamlOut, err := rm.AsYaml() + if err != nil { + return buildResult{Err: err.Error()} + } + return buildResult{YAML: string(yamlOut)} +} diff --git a/pkg/resolver/kustomize/helper_main_test.go b/pkg/resolver/kustomize/helper_main_test.go new file mode 100644 index 000000000..6aa2a9d1b --- /dev/null +++ b/pkg/resolver/kustomize/helper_main_test.go @@ -0,0 +1,13 @@ +package kustomize + +import ( + "os" + "testing" +) + +// TestMain mirrors cmd/scanner main: subprocess kustomize render re-execs this test binary with +// GO_WANT_KUSTOMIZE_RENDER_HELPER=1 and must handle stdin before tests run. +func TestMain(m *testing.M) { + MaybeRunAsKustomizeRenderHelper() + os.Exit(m.Run()) +} diff --git a/pkg/resolver/kustomize/resolver.go b/pkg/resolver/kustomize/resolver.go new file mode 100644 index 000000000..01ad250c5 --- /dev/null +++ b/pkg/resolver/kustomize/resolver.go @@ -0,0 +1,424 @@ +package kustomize + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/DataDog/datadog-iac-scanner/pkg/logger" + "github.com/DataDog/datadog-iac-scanner/pkg/model" + "github.com/DataDog/datadog-iac-scanner/pkg/resolver/sandbox" + "sigs.k8s.io/kustomize/api/provider" + "sigs.k8s.io/kustomize/api/resmap" + "sigs.k8s.io/kustomize/api/resource" +) + +// Options configures the Kustomize preprocessor. +type Options struct { + RepoRoot string + AllowHelmInflation bool + RenderTimeout time.Duration + HelmIncludeCRDs bool + // MaxFetchBytes: cap bytes under the resolve scratch dir (0 = unlimited). + MaxFetchBytes int64 + // StrictLoad: RootOnly (stricter) vs None (overlay-friendly, wider reads). + StrictLoad bool +} + +// Resolver renders kustomization directories to concrete manifests. +type Resolver struct { + opts Options +} + +// NewResolver constructs a Kustomize preprocessor. +func NewResolver(opts Options) *Resolver { + if opts.RenderTimeout <= 0 { + opts.RenderTimeout = 60 * time.Second + } + return &Resolver{opts: opts} +} + +// Name implements resolver.Preprocessor. +func (r *Resolver) Name() string { + return "kustomize" +} + +// Detect implements resolver.Preprocessor. +func (r *Resolver) Detect(path string) (model.FileKind, bool) { + return Detect(path) +} + +// SupportedTypes implements resolver.Preprocessor. +func (r *Resolver) SupportedTypes() []model.FileKind { + return []model.FileKind{model.KindKUSTOMIZE} +} + +// Resolve runs kustomize build for the given root directory. +func (r *Resolver) Resolve(ctx context.Context, rootDir string) (model.ResolvedFiles, error) { + contextLogger := logger.FromContext(ctx) + rootAbs, err := normalizeKustomRoot(rootDir) + if err != nil { + return model.ResolvedFiles{ + Diagnostics: []model.ResolverDiagnostic{{ + FilePath: rootDir, + Message: err.Error(), + QueryID: "kustomize-render-failed", + Line: 1, + }}, + }, nil + } + repoRoot := r.effectiveRepoRoot(rootAbs) + if repoRoot == "" { + repoRoot = rootAbs + } + repoAbs, err := filepath.Abs(repoRoot) + if err != nil { + return model.ResolvedFiles{}, err + } + if !isUnderRoot(rootAbs, repoAbs) { + return model.ResolvedFiles{ + Diagnostics: []model.ResolverDiagnostic{{ + FilePath: rootDir, + Message: "kustomization root is outside configured repo root", + QueryID: "kustomize-render-failed", + Line: 1, + }}, + }, nil + } + + sb, err := sandbox.New() + if err != nil { + return model.ResolvedFiles{}, err + } + defer func() { _ = sb.Close() }() + + kf := kustomizationEntryFile(rootAbs) + kustPath := filepath.Join(rootAbs, kf) + excluded, _ := TransitiveLocalPaths(rootAbs) + var diags []model.ResolverDiagnostic + remoteDiags, earlyRemote, stopRemote := remotePolicyResult(rootAbs, kustPath, excluded) + diags = append(diags, remoteDiags...) + if stopRemote { + return earlyRemote, nil + } + + br, fsRoot, d, err := PrepareHelmChartsIfNeeded( + ctx, repoAbs, rootAbs, sb.ScratchDir, + r.opts.AllowHelmInflation, r.opts.HelmIncludeCRDs, r.opts.MaxFetchBytes, + ) + if errors.Is(err, ErrMaxStagingBytes) { + return resolvedFilesWithDiagnostics(excluded, append(diags, d...)...), nil + } + if err != nil { + contextLogger.Warn().Msgf("kustomize helm prepass: %v", err) + } + buildRoot := br + runFSRoot := fsRoot + diags = append(diags, d...) + + scratchAbs := filepath.Clean(sb.ScratchDir) + brMeta, fsMeta, diags, earlyMeta, stopMeta := maybeStageBuildMetadata( + ctx, repoAbs, rootDir, scratchAbs, excluded, buildRoot, runFSRoot, diags, + ) + if stopMeta { + return earlyMeta, nil + } + buildRoot, runFSRoot = brMeta, fsMeta + + runCtx, cancel := context.WithTimeout(ctx, r.opts.RenderTimeout) + defer cancel() + + yamlOut, err := renderWithTimeout(runCtx, buildRoot, runFSRoot, r.opts.StrictLoad) + if err != nil { + if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) { + return resolvedFilesWithDiagnostics(excluded, append(diags, model.ResolverDiagnostic{ + FilePath: rootDir, + Message: "kustomize render timed out", + QueryID: "kustomize-render-failed", + Line: 1, + })...), nil + } + return resolvedFilesWithDiagnostics(excluded, append(diags, model.ResolverDiagnostic{ + FilePath: rootDir, + Message: err.Error(), + QueryID: "kustomize-render-failed", + Line: 1, + })...), nil + } + + rm, err := parseRenderedResMap(yamlOut) + if err != nil { + return resolvedFilesWithDiagnostics(excluded, append(diags, model.ResolverDiagnostic{ + FilePath: rootDir, + Message: err.Error(), + QueryID: "kustomize-render-failed", + Line: 1, + })...), nil + } + + if rf, stop := rejectIfScratchExceedsLimit(r.opts.MaxFetchBytes, sb.ScratchDir, rootDir, excluded, diags); stop { + return rf, nil + } + + out := resolvedVirtualsFromResMap(rm, rootAbs, repoAbs, scratchAbs, diags, excluded) + return out, nil +} + +func (r *Resolver) repoRootLimit(rootAbs string) string { + limit := "" + if r.opts.RepoRoot != "" { + if abs, err := filepath.Abs(r.opts.RepoRoot); err == nil { + limit = filepath.Clean(abs) + } else { + limit = filepath.Clean(r.opts.RepoRoot) + } + } + if gitRoot := nearestGitRoot(rootAbs); gitRoot != "" { + if limit == "" || isUnderRoot(gitRoot, limit) { + limit = gitRoot + } + } + return limit +} + +func (r *Resolver) effectiveRepoRoot(rootDir string) string { + rootAbs, err := filepath.Abs(rootDir) + if err != nil { + rootAbs = filepath.Clean(rootDir) + } + limit := r.repoRootLimit(rootAbs) + cur := rootAbs + best := rootAbs + for { + rels, err := TransitiveRelativeLocalPaths(cur) + if err != nil || len(rels) == 0 { + break + } + candidate := commonAncestorPathForRoots(rels) + if candidate == "" || filepath.Clean(candidate) == filepath.Clean(best) { + break + } + if limit != "" && !isUnderRoot(candidate, limit) { + break + } + best = candidate + cur = candidate + } + if limit != "" && isUnderRoot(best, limit) { + return best + } + if limit != "" { + return limit + } + return best +} + +func sourcePathForOutput(origin *model.KustomizeOrigin, kustomRoot string, res *resource.Resource) string { + if origin != nil && origin.SourceFile != "" && filepath.IsAbs(origin.SourceFile) { + return origin.SourceFile + } + if origin != nil && origin.SourceFile != "" && origin.SourceRepo != "" { + return origin.SourceFile + } + if origin != nil && origin.SourceFile != "" { + return filepath.Clean(filepath.Join(kustomRoot, origin.SourceFile)) + } + return filepath.Join(kustomRoot, "generated-"+res.GetGvk().Kind+"-"+res.GetName()+".yaml") +} + +func metadataPathForResolvedResource(origin *model.KustomizeOrigin, fallback string) string { + if origin == nil { + return fallback + } + if origin.OriginKind == model.KustomizeOriginTransformer { + for i := len(origin.Transformations) - 1; i >= 0; i-- { + if path := origin.Transformations[i].TransformerPath; path != "" && !looksLikeRemotePath(path) { + return cleanLocalPath(path) + } + } + } + return fallback +} + +// renderWithTimeout runs kustomize in a subprocess so CommandContext can kill +// it on deadline. Prod: same binary as `internal kustomize-render`; tests: same +// test binary with helperEnvVar + MaybeRunAsKustomizeRenderHelper. JSON over stdin/stdout. +func renderWithTimeout(ctx context.Context, buildRoot, runFSRoot string, strictLoad bool) (string, error) { + argv, extraEnv := helperInvocation() + bin, binErr := os.Executable() + if binErr != nil { + return "", fmt.Errorf("kustomize render helper binary: %w", binErr) + } + var cmd *exec.Cmd + if len(argv) == 0 { + //nolint:gosec // G204: bin is this process's own executable (os.Executable); no user input. + cmd = exec.CommandContext(ctx, bin) + } else { + //nolint:gosec // G204: bin is os.Executable(); argv is allowlisted in helperInvocation. + cmd = exec.CommandContext(ctx, bin, argv...) + } + cmd.Env = append(os.Environ(), extraEnv...) + + reqBytes, err := json.Marshal(helperRequest{ + BuildRoot: buildRoot, + RunFSRoot: runFSRoot, + StrictLoad: strictLoad, + }) + if err != nil { + return "", fmt.Errorf("encode kustomize render request: %w", err) + } + cmd.Stdin = bytes.NewReader(reqBytes) + + var stderr bytes.Buffer + cmd.Stderr = &stderr + + out, err := cmd.Output() + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return "", ctxErr + } + if msg := strings.TrimSpace(stderr.String()); msg != "" { + return "", fmt.Errorf("kustomize render helper: %s", msg) + } + return "", fmt.Errorf("kustomize render helper: %w", err) + } + + var result buildResult + if err := json.Unmarshal(bytes.TrimSpace(out), &result); err != nil { + return "", fmt.Errorf("decode kustomize render result: %w", err) + } + if result.Err != "" { + return "", errors.New(result.Err) + } + return result.YAML, nil +} + +// helperInvocation: prod uses `internal kustomize-render`; tests set helperEnvVar only (main calls MaybeRunAsKustomizeRenderHelper). +func helperInvocation() (argv, env []string) { + if isTestBinary() { + return nil, []string{helperEnvVar + "=1"} + } + return []string{"internal", "kustomize-render"}, nil +} + +func isTestBinary() bool { + base := filepath.Base(os.Args[0]) + return strings.HasSuffix(base, ".test") || strings.HasSuffix(base, ".test.exe") +} + +func parseRenderedResMap(yamlOut string) (resmap.ResMap, error) { + return resmap.NewFactory(provider.NewDepProvider().GetResourceFactory()).NewResMapFromBytes([]byte(yamlOut)) +} + +func skipLocalConfig(res *resource.Resource) bool { + ann := res.GetAnnotations() + if ann == nil { + return false + } + return ann["config.kubernetes.io/local-config"] == "true" +} + +func commonAncestorPathForRoots(paths []string) string { + if len(paths) == 0 { + return "" + } + common := filepath.Clean(paths[0]) + for _, p := range paths[1:] { + cur := filepath.Clean(p) + for common != cur && !strings.HasPrefix(cur, common+string(filepath.Separator)) { + parent := filepath.Dir(common) + if parent == common { + return common + } + common = parent + } + } + return common +} + +func nearestGitRoot(path string) string { + cur := filepath.Clean(path) + for { + if _, err := os.Stat(filepath.Join(cur, ".git")); err == nil { + return cur + } + parent := filepath.Dir(cur) + if parent == cur { + return "" + } + cur = parent + } +} + +func resolvedFilesWithDiagnostics(excluded []string, diags ...model.ResolverDiagnostic) model.ResolvedFiles { + return model.ResolvedFiles{ + Excluded: stringSliceUnique(excluded), + Diagnostics: diags, + } +} + +func normalizeKustomRoot(rootDir string) (string, error) { + rootAbs, err := filepath.Abs(rootDir) + if err != nil { + return "", err + } + st, err := os.Stat(rootAbs) + if err != nil { + return "", err + } + if st.IsDir() { + return rootAbs, nil + } + if IsKustomizationEntryFile(rootAbs) { + return filepath.Dir(rootAbs), nil + } + return "", fmt.Errorf("kustomize root %q is neither a directory nor a kustomization entry file", rootDir) +} + +func stringSliceUnique(in []string) []string { + seen := make(map[string]struct{}) + var out []string + for _, s := range in { + if s == "" { + continue + } + if _, ok := seen[s]; ok { + continue + } + seen[s] = struct{}{} + out = append(out, s) + } + return out +} + +// originalDataForResolvedResource returns the bytes to expose as the source +// for a rendered resource. Local origin paths are read only when they sit +// under repoAbs or scratchAbs to avoid leaking arbitrary host files via +// attacker-controlled kustomize origin annotations. +func originalDataForResolvedResource( + origin *model.KustomizeOrigin, + renderedYAML string, + repoAbs, scratchAbs string, +) []byte { + if origin == nil { + return []byte(renderedYAML) + } + if origin.SourceFile != "" && origin.SourceRepo == "" && filepath.IsAbs(origin.SourceFile) { + if raw, ok := safeReadResolvedFile(origin.SourceFile, repoAbs, scratchAbs); ok { + return raw + } + } + if origin.OriginalSourceFile != "" && origin.OriginalSourceRepo == "" && filepath.IsAbs(origin.OriginalSourceFile) { + if raw, ok := safeReadResolvedFile(origin.OriginalSourceFile, repoAbs, scratchAbs); ok { + return raw + } + } + return []byte(renderedYAML) +} diff --git a/pkg/resolver/kustomize/resolver_phases.go b/pkg/resolver/kustomize/resolver_phases.go new file mode 100644 index 000000000..d672c71b6 --- /dev/null +++ b/pkg/resolver/kustomize/resolver_phases.go @@ -0,0 +1,273 @@ +package kustomize + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/DataDog/datadog-iac-scanner/pkg/logger" + "github.com/DataDog/datadog-iac-scanner/pkg/model" + "github.com/DataDog/datadog-iac-scanner/pkg/rootfile" + "gopkg.in/yaml.v3" + "sigs.k8s.io/kustomize/api/resmap" +) + +func rejectIfScratchExceedsLimit( + maxBytes int64, + scratchDir, rootDir string, + excluded []string, + diags []model.ResolverDiagnostic, +) (model.ResolvedFiles, bool) { + if maxBytes <= 0 { + return model.ResolvedFiles{}, false + } + sz, szErr := DirTotalSize(scratchDir) + if szErr != nil || sz <= maxBytes { + return model.ResolvedFiles{}, false + } + return resolvedFilesWithDiagnostics(excluded, append(diags, model.ResolverDiagnostic{ + FilePath: rootDir, + Message: fmt.Sprintf( + "kustomize scratch dir size exceeds configured maximum (%d > %d bytes)", sz, maxBytes, + ), + QueryID: "kustomize-max-fetch-exceeded", + Line: 1, + })...), true +} + +// remotePolicyWalk scans the kustomization tree for remote refs and inline KRM diagnostics. +func remotePolicyWalk(rootAbs string) ( + firstRemotePath string, + firstRemoteRefs []string, + krmDiags []model.ResolverDiagnostic, + walkErr error, +) { + walkErr = WalkLocalKustomizations(rootAbs, func(kustPath string, raw []byte) error { + if rem, err := CollectRemoteRefsFromKustomization(raw); err == nil && len(rem) > 0 && firstRemotePath == "" { + firstRemotePath = kustPath + firstRemoteRefs = append([]string(nil), rem...) + } + var doc map[string]interface{} + if err := yaml.Unmarshal(raw, &doc); err == nil && DetectKRMInlineFunctions(doc) { + krmDiags = append(krmDiags, model.ResolverDiagnostic{ + FilePath: kustPath, + Message: "inline KRM generators/transformers/validators are not executed in this scanner", + QueryID: "kustomize-exec-plugin-disabled", + Line: 1, + }) + } + return nil + }) + return firstRemotePath, firstRemoteRefs, krmDiags, walkErr +} + +func remotePolicyResult(rootAbs, kustPath string, excluded []string) ([]model.ResolverDiagnostic, model.ResolvedFiles, bool) { + firstRemotePath, firstRemoteRefs, krmDiags, remotePolicyErr := remotePolicyWalk(rootAbs) + if remotePolicyErr == nil && len(firstRemoteRefs) > 0 { + return nil, resolvedFilesWithDiagnostics(excluded, model.ResolverDiagnostic{ + FilePath: firstRemotePath, + Message: fmt.Sprintf("kustomization references remote resources which are not supported: %v", firstRemoteRefs), + QueryID: "kustomize-remote-disallowed", + Line: 1, + }), true + } + return appendKRMIfWalkFailed(kustPath, remotePolicyErr, krmDiags), model.ResolvedFiles{}, false +} + +func appendKRMIfWalkFailed(kustPath string, remotePolicyErr error, diags []model.ResolverDiagnostic) []model.ResolverDiagnostic { + if remotePolicyErr == nil { + return diags + } + if raw, err := rootfile.ReadFile(kustPath); err == nil { + var doc map[string]interface{} + if err := yaml.Unmarshal(raw, &doc); err == nil && DetectKRMInlineFunctions(doc) { + diags = append(diags, model.ResolverDiagnostic{ + FilePath: kustPath, + Message: "inline KRM generators/transformers/validators are not executed in this scanner", + QueryID: "kustomize-exec-plugin-disabled", + Line: 1, + }) + } + } + return diags +} + +// maybeStageBuildMetadata returns done=true and rf when Resolve should return rf immediately. +func maybeStageBuildMetadata( + ctx context.Context, + repoAbs, rootDir, scratchAbs string, + excluded []string, + buildRoot, runFSRoot string, + diags []model.ResolverDiagnostic, +) (brOut, fsOut string, diagsOut []model.ResolverDiagnostic, rf model.ResolvedFiles, done bool) { + contextLogger := logger.FromContext(ctx) + buildAbs := filepath.Clean(buildRoot) + kustRel := kustomizationEntryFile(buildAbs) + metaPath := filepath.Join(buildAbs, kustRel) + metaBytes, err := rootfile.ReadFile(metaPath) + if err != nil { + return buildRoot, runFSRoot, diags, model.ResolvedFiles{}, false + } + need, err := buildMetadataSupplementsNeeded(metaBytes) + if err != nil { + contextLogger.Warn().Msgf("kustomize buildMetadata read: %v", err) + return buildRoot, runFSRoot, diags, model.ResolvedFiles{}, false + } + if !need { + return buildRoot, runFSRoot, diags, model.ResolvedFiles{}, false + } + if isUnderRoot(buildAbs, scratchAbs) { + if err := ensureBuildMetadataIfNeeded(buildRoot); err != nil { + contextLogger.Warn().Msgf("kustomize buildMetadata: %v", err) + } + return buildRoot, runFSRoot, diags, model.ResolvedFiles{}, false + } + relFromRepo, relErr := filepath.Rel(repoAbs, buildAbs) + if relErr != nil || relFromRepo == relDotDot || strings.HasPrefix(relFromRepo, parentDirPrefix) { + return "", "", nil, resolvedFilesWithDiagnostics(excluded, append(diags, model.ResolverDiagnostic{ + FilePath: rootDir, + Message: fmt.Sprintf("kustomize root %q is outside repo root %q; cannot stage safely for buildMetadata", buildAbs, repoAbs), + QueryID: "kustomize-render-failed", + Line: 1, + })...), true + } + sum := sha256.Sum256([]byte(repoAbs + "\x00" + buildAbs)) + stagedRepo := filepath.Join(scratchAbs, "kustomize-build", hex.EncodeToString(sum[:])) + _ = os.RemoveAll(stagedRepo) + stageRels, stageErr := BuildMetadataStageRelPaths(repoAbs, buildAbs) + if stageErr != nil { + return "", "", nil, resolvedFilesWithDiagnostics(excluded, append(diags, model.ResolverDiagnostic{ + FilePath: rootDir, + Message: fmt.Sprintf("kustomize buildMetadata staging paths: %v", stageErr), + QueryID: "kustomize-render-failed", + Line: 1, + })...), true + } + if err := CopyRepoRelativeFilesNoSymlinks(stagedRepo, repoAbs, stageRels); err != nil { + return "", "", nil, resolvedFilesWithDiagnostics(excluded, append(diags, model.ResolverDiagnostic{ + FilePath: rootDir, + Message: fmt.Sprintf("kustomize staging for buildMetadata: %v", err), + QueryID: "kustomize-render-failed", + Line: 1, + })...), true + } + stagedKustom := filepath.Join(stagedRepo, relFromRepo) + if err := ensureBuildMetadataIfNeeded(stagedKustom); err != nil { + contextLogger.Warn().Msgf("kustomize buildMetadata: %v", err) + } + return stagedKustom, stagedRepo, diags, model.ResolvedFiles{}, false +} + +func appendMissingTransformerPathDiags( + origin *model.KustomizeOrigin, + repoAbs, scratchAbs string, + seen map[string]struct{}, + diags []model.ResolverDiagnostic, +) []model.ResolverDiagnostic { + if origin == nil || len(origin.Transformations) == 0 { + return diags + } + for _, tr := range origin.Transformations { + if tr.TransformerPath == "" || strings.Contains(tr.TransformerPath, "://") { + continue + } + // Skip paths outside the repo/scratch sandbox so attacker-controlled + // origin annotations don't probe host files or leak into diagnostics. + if !isResolvedFileSafe(tr.TransformerPath, repoAbs, scratchAbs) { + continue + } + if _, err := rootfile.Lstat(tr.TransformerPath); err != nil { + key := origin.GeneratorConfigFile + "\x00" + tr.FieldPath + "\x00" + tr.TransformerPath + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + diags = append(diags, model.ResolverDiagnostic{ + FilePath: origin.GeneratorConfigFile, + Message: fmt.Sprintf( + "transformer patch not found at %q (declared path %q)", + tr.TransformerPath, tr.FieldPath, + ), + QueryID: "kustomize-transformer-path-missing", + Line: 1, + }) + } + } + return diags +} + +// isResolvedFileSafe reports whether a kustomize-supplied path may be read as +// scan source bytes. It rejects empty/remote/relative paths and any path that +// is not under one of the configured roots (repo or scratch). +func isResolvedFileSafe(path string, roots ...string) bool { + if path == "" || strings.Contains(path, "://") || !filepath.IsAbs(path) { + return false + } + clean := filepath.Clean(path) + for _, r := range roots { + if r != "" && isUnderRoot(clean, r) { + return true + } + } + return false +} + +// safeReadResolvedFile reads path through rootfile only when it satisfies +// isResolvedFileSafe. Returns ok=false on any rejection or read error. +func safeReadResolvedFile(path string, roots ...string) ([]byte, bool) { + if !isResolvedFileSafe(path, roots...) { + return nil, false + } + raw, err := rootfile.ReadFile(filepath.Clean(path)) + if err != nil { + return nil, false + } + return raw, true +} + +func resolvedVirtualsFromResMap( + rm resmap.ResMap, + rootAbs, repoAbs, scratchAbs string, + diags []model.ResolverDiagnostic, + excluded []string, +) model.ResolvedFiles { + var out model.ResolvedFiles + out.Excluded = stringSliceUnique(excluded) + missingTransformerDiag := map[string]struct{}{} + for _, res := range rm.Resources() { + if skipLocalConfig(res) { + continue + } + yml := res.MustYaml() + origin := OriginFromResource(res, rootAbs) + diags = appendMissingTransformerPathDiags(origin, repoAbs, scratchAbs, missingTransformerDiag, diags) + if origin != nil && origin.SourceFile != "" && origin.SourceRepo == "" && !filepath.IsAbs(origin.SourceFile) { + origin.SourceFile = filepath.Join(rootAbs, origin.SourceFile) + } + srcPath := sourcePathForOutput(origin, rootAbs, res) + metadataPath := metadataPathForResolvedResource(origin, srcPath) + orig := originalDataForResolvedResource(origin, yml, repoAbs, scratchAbs) + if metadataPath != "" { + if raw, ok := safeReadResolvedFile(metadataPath, repoAbs, scratchAbs); ok { + orig = raw + } else if metadataPath != srcPath { + // Untrusted transformer path: don't surface it to downstream + // parsers/sinks; fall back to the rendered source path. + metadataPath = srcPath + } + } + out.File = append(out.File, model.ResolvedVirtual{ + FileName: srcPath, + MetadataPath: metadataPath, + Content: []byte(yml), + OriginalData: orig, + Origin: origin, + }) + } + out.Diagnostics = diags + return out +} diff --git a/pkg/resolver/kustomize/resolver_test.go b/pkg/resolver/kustomize/resolver_test.go new file mode 100644 index 000000000..7a897dc40 --- /dev/null +++ b/pkg/resolver/kustomize/resolver_test.go @@ -0,0 +1,610 @@ +package kustomize + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/DataDog/datadog-iac-scanner/pkg/model" + "github.com/stretchr/testify/require" +) + +func TestResolver_KRMInlineEmitsDiagnostic(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + require.NoError(t, writeFile(filepath.Join(dir, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +generators: +- apiVersion: builtin + kind: ConfigMapGenerator + name: gen +resources: [] +`)) + r := NewResolver(Options{RepoRoot: dir}) + out, err := r.Resolve(ctx, dir) + require.NoError(t, err) + var found bool + for _, d := range out.Diagnostics { + if d.QueryID == "kustomize-exec-plugin-disabled" { + found = true + break + } + } + require.True(t, found, "expected kustomize-exec-plugin-disabled diagnostic") +} + +func TestResolver_RemoteRefsDisallowed(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + require.NoError(t, writeFile(filepath.Join(dir, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- https://example.com/some/base +`)) + r := NewResolver(Options{RepoRoot: dir}) + out, err := r.Resolve(ctx, dir) + require.NoError(t, err) + require.Empty(t, out.File) + require.Len(t, out.Diagnostics, 1) + require.Equal(t, "kustomize-remote-disallowed", out.Diagnostics[0].QueryID) + require.Contains(t, out.Excluded, filepath.Join(dir, "kustomization.yaml")) +} + +func TestResolver_LegacyHostStyleRemoteRefDisallowed(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + require.NoError(t, writeFile(filepath.Join(dir, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- github.com/Liujingfang1/mysql?ref=test +`)) + r := NewResolver(Options{RepoRoot: dir}) + out, err := r.Resolve(ctx, dir) + require.NoError(t, err) + require.Empty(t, out.File) + require.Len(t, out.Diagnostics, 1) + require.Equal(t, "kustomize-remote-disallowed", out.Diagnostics[0].QueryID) + require.Contains(t, out.Diagnostics[0].Message, "github.com/Liujingfang1/mysql?ref=test") +} + +func TestResolver_HelmChartsRemoteRepoNotPreRejectedWhenInflationDisabled(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + require.NoError(t, writeFile(filepath.Join(repo, "local.yaml"), `apiVersion: v1 +kind: ConfigMap +metadata: + name: keep-me +data: + k: v +`)) + require.NoError(t, writeFile(filepath.Join(repo, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- local.yaml +helmCharts: +- name: mini + repo: https://charts.example.com + releaseName: rel +`)) + + r := NewResolver(Options{RepoRoot: repo, AllowHelmInflation: false}) + out, err := r.Resolve(ctx, repo) + require.NoError(t, err) + + var sawInflationDisabled bool + for _, d := range out.Diagnostics { + require.NotEqual(t, "kustomize-remote-disallowed", d.QueryID, "%+v", d) + if d.QueryID == "kustomize-helm-inflation-disabled" { + sawInflationDisabled = true + } + } + require.True(t, sawInflationDisabled, "%+v", out.Diagnostics) + + var sawLocal bool + for _, f := range out.File { + if strings.Contains(f.FileName, "local.yaml") { + sawLocal = true + } + } + require.True(t, sawLocal, "%+v", out.File) +} + +func TestResolver_RejectsLocalPathsOutsideRepoRoot(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + outside := t.TempDir() + require.NoError(t, writeFile(filepath.Join(repo, "kustomization.yaml"), "apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\nresources:\n- "+filepath.Join(outside, "deployment.yaml")+"\n")) + + r := NewResolver(Options{RepoRoot: repo}) + out, err := r.Resolve(ctx, repo) + require.NoError(t, err) + require.Empty(t, out.File) + require.NotEmpty(t, out.Diagnostics) + require.Equal(t, "kustomize-render-failed", out.Diagnostics[len(out.Diagnostics)-1].QueryID) +} + +func TestResolver_RemoteRefsDisallowedInNestedLocalBase(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + base := filepath.Join(repo, "base") + root := filepath.Join(repo, "root") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(root, 0o700)) + require.NoError(t, writeFile(filepath.Join(base, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- https://example.com/nested/remote +`)) + require.NoError(t, writeFile(filepath.Join(root, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +`)) + + r := NewResolver(Options{RepoRoot: repo}) + out, err := r.Resolve(ctx, root) + require.NoError(t, err) + require.Empty(t, out.File) + require.Len(t, out.Diagnostics, 1) + require.Equal(t, "kustomize-remote-disallowed", out.Diagnostics[0].QueryID) + require.Equal(t, filepath.Join(base, "kustomization.yaml"), out.Diagnostics[0].FilePath) +} + +func TestResolver_NamespaceFromOverlay(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + + require.NoError(t, writeFile(filepath.Join(base, "deployment.yaml"), `apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + selector: + matchLabels: + app: app + template: + metadata: + labels: + app: app + spec: + containers: + - name: app + image: nginx +`)) + require.NoError(t, writeFile(filepath.Join(base, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- deployment.yaml +`)) + require.NoError(t, writeFile(filepath.Join(overlay, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +namespace: production +`)) + + r := NewResolver(Options{ + RepoRoot: repo, + AllowHelmInflation: false, + RenderTimeout: 0, + HelmIncludeCRDs: true, + }) + out, err := r.Resolve(ctx, overlay) + require.NoError(t, err) + require.Empty(t, out.Diagnostics, "%+v", out.Diagnostics) + require.NotEmpty(t, out.File) + + var joined strings.Builder + for _, f := range out.File { + joined.Write(f.Content) + } + combined := joined.String() + require.Contains(t, combined, "namespace: production") +} + +func TestResolver_NamespaceFromOverlay_FileInputPath(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + + require.NoError(t, writeFile(filepath.Join(base, "deployment.yaml"), `apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + selector: + matchLabels: + app: app + template: + metadata: + labels: + app: app + spec: + containers: + - name: app + image: nginx +`)) + require.NoError(t, writeFile(filepath.Join(base, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- deployment.yaml +`)) + require.NoError(t, writeFile(filepath.Join(overlay, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +namespace: production +`)) + + r := NewResolver(Options{ + RepoRoot: repo, + AllowHelmInflation: false, + RenderTimeout: 0, + HelmIncludeCRDs: true, + }) + out, err := r.Resolve(ctx, filepath.Join(overlay, "kustomization.yaml")) + require.NoError(t, err) + require.Empty(t, out.Diagnostics, "%+v", out.Diagnostics) + require.NotEmpty(t, out.File) + + var joined strings.Builder + for _, f := range out.File { + joined.Write(f.Content) + } + combined := joined.String() + require.Contains(t, combined, "namespace: production") +} + +func TestResolver_TransformerConfiguredInTracksBaseKustomization(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + + require.NoError(t, writeFile(filepath.Join(base, "deployment.yaml"), `apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + replicas: 1 + selector: + matchLabels: + app: app + template: + metadata: + labels: + app: app + spec: + containers: + - name: app + image: nginx +`)) + require.NoError(t, writeFile(filepath.Join(base, "patch.yaml"), `apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + replicas: 5 +`)) + require.NoError(t, writeFile(filepath.Join(base, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- deployment.yaml +patches: +- path: patch.yaml +`)) + require.NoError(t, writeFile(filepath.Join(overlay, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +`)) + + r := NewResolver(Options{ + RepoRoot: repo, + AllowHelmInflation: false, + HelmIncludeCRDs: true, + }) + out, err := r.Resolve(ctx, overlay) + require.NoError(t, err) + require.Empty(t, out.Diagnostics, "%+v", out.Diagnostics) + require.NotEmpty(t, out.File) + + found := false + for _, f := range out.File { + if f.Origin == nil || f.Origin.OriginKind != model.KustomizeOriginTransformer { + continue + } + require.NotEmpty(t, f.Origin.Transformations) + require.Equal(t, filepath.Join(base, "kustomization.yaml"), f.Origin.Transformations[0].ConfiguredIn) + found = true + } + require.True(t, found, "expected transformed resource with base-owned configuredIn path") +} + +func TestResolver_DirectOriginPreservesSourceBytes(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + root := filepath.Join(repo, "root") + require.NoError(t, os.MkdirAll(root, 0o700)) + sourcePath := filepath.Join(root, "deployment.yaml") + source := `# dd-iac-scan ignore-block +apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + replicas: 1 +` + require.NoError(t, writeFile(sourcePath, source)) + require.NoError(t, writeFile(filepath.Join(root, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- deployment.yaml +`)) + + r := NewResolver(Options{RepoRoot: repo}) + out, err := r.Resolve(ctx, root) + require.NoError(t, err) + require.NotEmpty(t, out.File) + require.Equal(t, sourcePath, out.File[0].FileName) + require.Equal(t, source, string(out.File[0].OriginalData)) +} + +func TestResolver_TransformerOriginPreservesOriginalSourceBytes(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + source := `# dd-iac-scan ignore-block +apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + replicas: 1 +` + require.NoError(t, writeFile(filepath.Join(base, "deployment.yaml"), source)) + require.NoError(t, writeFile(filepath.Join(overlay, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base/deployment.yaml +namespace: prod +`)) + + r := NewResolver(Options{RepoRoot: repo}) + out, err := r.Resolve(ctx, overlay) + require.NoError(t, err) + require.NotEmpty(t, out.File) + require.Equal(t, source, string(out.File[0].OriginalData)) + require.NotNil(t, out.File[0].Origin) + require.Equal(t, model.KustomizeOriginTransformer, out.File[0].Origin.OriginKind) +} + +func TestRenderWithTimeout_KillsHelperProcess(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + + t.Setenv("KUSTOMIZE_HELPER_SLEEP_MS", "200") + _, err := renderWithTimeout(ctx, t.TempDir(), t.TempDir(), false) + require.Error(t, err) + require.ErrorIs(t, err, context.DeadlineExceeded) +} + +func TestResolver_ExcludedIncludesKustomizationAndUpwardBaseRefs(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + require.NoError(t, writeFile(filepath.Join(base, "deployment.yaml"), `apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + replicas: 1 +`)) + require.NoError(t, writeFile(filepath.Join(base, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- deployment.yaml +`)) + require.NoError(t, writeFile(filepath.Join(overlay, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +`)) + + r := NewResolver(Options{RepoRoot: repo}) + out, err := r.Resolve(ctx, overlay) + require.NoError(t, err) + require.Contains(t, out.Excluded, filepath.Join(overlay, "kustomization.yaml")) + require.Contains(t, out.Excluded, filepath.Join(base)) +} + +func TestSourcePathForOutput_PreservesRemoteOriginPath(t *testing.T) { + origin := &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginDirect, + SourceFile: "https://github.com/org/repo/base/deployment.yaml", + SourceRepo: "https://github.com/org/repo", + } + got := sourcePathForOutput(origin, "/tmp/local-root", nil) + require.Equal(t, "https://github.com/org/repo/base/deployment.yaml", got) +} + +func TestCleanLocalPath_PreservesRemoteTransformerURL(t *testing.T) { + got := cleanLocalPath("https://github.com/org/repo/base/patch.yaml") + require.Equal(t, "https://github.com/org/repo/base/patch.yaml", got) +} + +func TestResolver_EffectiveRepoRoot_StaysWithinNeededSubtreeInsideGitRepo(t *testing.T) { + repo := t.TempDir() + overlay := filepath.Join(repo, "services", "app", "overlay") + base := filepath.Join(repo, "services", "base") + other := filepath.Join(repo, "other") + require.NoError(t, os.MkdirAll(filepath.Join(repo, ".git"), 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(other, 0o700)) + require.NoError(t, writeFile(filepath.Join(overlay, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../../base +`)) + + r := NewResolver(Options{RepoRoot: repo}) + got := r.effectiveRepoRoot(overlay) + require.Equal(t, filepath.Join(repo, "services"), got) +} + +func TestResolver_EffectiveRepoRoot_DoesNotBroadenAcrossDistinctGitRepos(t *testing.T) { + parent := t.TempDir() + repoA := filepath.Join(parent, "repo-a") + repoB := filepath.Join(parent, "repo-b") + overlayA := filepath.Join(repoA, "overlay") + require.NoError(t, os.MkdirAll(filepath.Join(repoA, ".git"), 0o700)) + require.NoError(t, os.MkdirAll(filepath.Join(repoB, ".git"), 0o700)) + require.NoError(t, os.MkdirAll(overlayA, 0o700)) + require.NoError(t, writeFile(filepath.Join(overlayA, "kustomization.yaml"), `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: [] +`)) + + r := NewResolver(Options{RepoRoot: parent}) + got := r.effectiveRepoRoot(overlayA) + require.Equal(t, overlayA, got) + require.NotEqual(t, parent, got) +} + +func TestDetect(t *testing.T) { + dir := t.TempDir() + require.NoError(t, writeFile(filepath.Join(dir, "kustomization.yaml"), "apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\n")) + k, ok := Detect(dir) + require.True(t, ok) + require.Equal(t, string(model.KindKUSTOMIZE), string(k)) +} + +func TestIsResolvedFileSafe(t *testing.T) { + repo := t.TempDir() + scratch := t.TempDir() + other := t.TempDir() + + cases := []struct { + name string + path string + want bool + }{ + {"empty", "", false}, + {"relative", "patch.yaml", false}, + {"remote URL", "https://example.com/patch.yaml", false}, + {"absolute outside roots", filepath.Join(other, "patch.yaml"), false}, + {"absolute under repo", filepath.Join(repo, "base", "patch.yaml"), true}, + {"absolute under scratch", filepath.Join(scratch, "kustomize-build", "x.yaml"), true}, + {"escapes via dotdot back into repo", filepath.Join(repo, "..", filepath.Base(repo), "x.yaml"), true}, + {"escapes via dotdot to host", filepath.Join(repo, "..", "..", "..", "etc", "passwd"), false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := isResolvedFileSafe(tc.path, repo, scratch) + require.Equal(t, tc.want, got) + }) + } +} + +func TestSafeReadResolvedFile_RejectsOutOfRoot(t *testing.T) { + repo := t.TempDir() + outside := t.TempDir() + target := filepath.Join(outside, "secret") + require.NoError(t, os.WriteFile(target, []byte("topsecret"), 0o600)) + + _, ok := safeReadResolvedFile(target, repo) + require.False(t, ok, "must not read host files outside configured roots") +} + +func TestSafeReadResolvedFile_AllowsInRoot(t *testing.T) { + repo := t.TempDir() + target := filepath.Join(repo, "patch.yaml") + require.NoError(t, os.WriteFile(target, []byte("payload"), 0o600)) + + raw, ok := safeReadResolvedFile(target, repo, "") + require.True(t, ok) + require.Equal(t, "payload", string(raw)) +} + +func TestOriginalDataForResolvedResource_DropsOutOfRootSourceFile(t *testing.T) { + repo := t.TempDir() + outside := t.TempDir() + hostFile := filepath.Join(outside, "passwd") + require.NoError(t, os.WriteFile(hostFile, []byte("root:x:0:0:root:/root:/bin/sh\n"), 0o600)) + + origin := &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginDirect, + SourceFile: hostFile, + } + rendered := "kind: ConfigMap\n" + got := originalDataForResolvedResource(origin, rendered, repo, "") + require.Equal(t, rendered, string(got), "must not surface host file bytes as OriginalData") +} + +func TestOriginalDataForResolvedResource_DropsOutOfRootOriginalSourceFile(t *testing.T) { + repo := t.TempDir() + outside := t.TempDir() + hostFile := filepath.Join(outside, "shadow") + require.NoError(t, os.WriteFile(hostFile, []byte("hash"), 0o600)) + + origin := &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginTransformer, + OriginalSourceFile: hostFile, + } + rendered := "kind: Deployment\n" + got := originalDataForResolvedResource(origin, rendered, repo, "") + require.Equal(t, rendered, string(got)) +} + +func TestAppendMissingTransformerPathDiags_SkipsOutOfRoot(t *testing.T) { + repo := t.TempDir() + outside := t.TempDir() + missingHostPath := filepath.Join(outside, "does-not-exist", "patch.yaml") + + origin := &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginTransformer, + GeneratorConfigFile: filepath.Join(repo, "kustomization.yaml"), + Transformations: []model.KustomizeTransformation{ + {TransformerPath: missingHostPath, FieldPath: "patches[0].path"}, + }, + } + got := appendMissingTransformerPathDiags(origin, repo, "", map[string]struct{}{}, nil) + require.Empty(t, got, "must not emit diagnostics that surface attacker-controlled host paths") +} + +func TestOriginalDataForResolvedResource_ReadsInRootSourceFile(t *testing.T) { + repo := t.TempDir() + srcPath := filepath.Join(repo, "deployment.yaml") + src := "kind: Deployment\nmetadata:\n name: app\n" + require.NoError(t, os.WriteFile(srcPath, []byte(src), 0o600)) + + origin := &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginDirect, + SourceFile: srcPath, + } + got := originalDataForResolvedResource(origin, "rendered", repo, "") + require.Equal(t, src, string(got)) +} + +func writeFile(p, content string) error { + return os.WriteFile(p, []byte(content), 0o600) +} From 4a4e4b60dc6b9ea763c449a9e1d30416d372c79f Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Wed, 29 Apr 2026 13:51:52 +0200 Subject: [PATCH 15/17] Add Kustomize detector with direct-source and generator/transformer line mapping. --- pkg/detector/kustomize/detect.go | 108 +++++ pkg/detector/kustomize/detect_test.go | 369 ++++++++++++++++++ pkg/detector/kustomize/direct_line.go | 211 ++++++++++ pkg/detector/kustomize/generator_line.go | 312 +++++++++++++++ pkg/detector/kustomize/generator_line_test.go | 35 ++ pkg/detector/kustomize/helper_main_test.go | 14 + pkg/detector/kustomize/namehash.go | 21 + pkg/detector/kustomize/namehash_test.go | 27 ++ 8 files changed, 1097 insertions(+) create mode 100644 pkg/detector/kustomize/detect.go create mode 100644 pkg/detector/kustomize/detect_test.go create mode 100644 pkg/detector/kustomize/direct_line.go create mode 100644 pkg/detector/kustomize/generator_line.go create mode 100644 pkg/detector/kustomize/generator_line_test.go create mode 100644 pkg/detector/kustomize/helper_main_test.go create mode 100644 pkg/detector/kustomize/namehash.go create mode 100644 pkg/detector/kustomize/namehash_test.go diff --git a/pkg/detector/kustomize/detect.go b/pkg/detector/kustomize/detect.go new file mode 100644 index 000000000..a1ccf8bca --- /dev/null +++ b/pkg/detector/kustomize/detect.go @@ -0,0 +1,108 @@ +package kustomize + +import ( + "context" + "path/filepath" + "strings" + + "github.com/DataDog/datadog-iac-scanner/pkg/detector" + "github.com/DataDog/datadog-iac-scanner/pkg/model" + "github.com/DataDog/datadog-iac-scanner/pkg/rootfile" +) + +// DetectKindLine maps Kustomize-rendered findings back to source lines. +type DetectKindLine struct{} + +// DetectLine: direct sources use default YAML tracing; generators/transformers use kustomization lines. +func (DetectKindLine) DetectLine( + ctx context.Context, + file *model.FileMetadata, + searchKey string, + outputLines int, +) model.VulnerabilityLines { + o := file.KustomizeOrigin + if o != nil && o.OriginKind == model.KustomizeOriginDirect && o.SourceFile != "" && o.SourceRepo == "" { + if v := directSourceDetectLine(o.SourceFile, searchKey, outputLines); v.Line > 0 { + return v + } + } + if o == nil || !o.RequiresDetailedLineMapping() { + return detector.DefaultYAMLDetectLine{}.DetectLine(ctx, file, searchKey, outputLines) + } + cfg := o.GeneratorConfigFile + if cfg == "" { + return detector.DefaultYAMLDetectLine{}.DetectLine(ctx, file, searchKey, outputLines) + } + data, err := rootfile.ReadFile(filepath.Clean(cfg)) + if err != nil { + return detector.DefaultYAMLDetectLine{}.DetectLine(ctx, file, searchKey, outputLines) + } + lines := strings.Split(string(data), "\n") + if v, ok := mappedLineFromKustomizeOrigin(ctx, file, o, searchKey, outputLines, lines); ok { + return v + } + return detector.DefaultYAMLDetectLine{}.DetectLine(ctx, file, searchKey, outputLines) +} + +func mappedLineFromKustomizeOrigin( + ctx context.Context, + file *model.FileMetadata, + o *model.KustomizeOrigin, + searchKey string, + outputLines int, + lines []string, +) (model.VulnerabilityLines, bool) { + switch o.OriginKind { + case model.KustomizeOriginGenerator: + name := o.ResourceName + if name == "" { + name = metadataNameFromDoc(file.Document) + } + line1, p := generatorConfigLine(o, name) + return buildMappedVulnLines(line1-1, p, lines, outputLines), true + case model.KustomizeOriginTransformer: + if v := transformerPatchFileLine(ctx, o, searchKey, outputLines); v.Line > 0 { + return v, true + } + if o.OriginalSourceFile != "" && o.OriginalSourceRepo == "" { + if v := directSourceDetectLine(o.OriginalSourceFile, searchKey, outputLines); v.Line > 0 { + return v, true + } + } + line1, p := transformerLineForOrigin(o) + return buildMappedVulnLines(line1-1, p, lines, outputLines), true + default: + return model.VulnerabilityLines{}, false + } +} + +func buildMappedVulnLines(lineIdx int, resolvedPath string, lines []string, outputLines int) model.VulnerabilityLines { + if lineIdx < 0 { + lineIdx = 0 + } + if lineIdx >= len(lines) { + lineIdx = 0 + } + return model.VulnerabilityLines{ + Line: lineIdx + 1, + VulnLines: detector.GetAdjacentVulnLines(lineIdx, outputLines, lines), + LineWithVulnerability: strings.TrimSpace(lines[lineIdx]), + ResolvedFile: resolvedPath, + VulnerablilityLocation: model.ResourceLocation{ + Start: model.ResourceLine{Line: lineIdx + 1, Col: 1}, + End: model.ResourceLine{Line: lineIdx + 1, Col: 1}, + }, + } +} + +func metadataNameFromDoc(doc model.Document) string { + if doc == nil { + return "" + } + m, ok := doc["metadata"].(map[string]interface{}) + if !ok { + return "" + } + n, _ := m["name"].(string) + return n +} diff --git a/pkg/detector/kustomize/detect_test.go b/pkg/detector/kustomize/detect_test.go new file mode 100644 index 000000000..198850eb2 --- /dev/null +++ b/pkg/detector/kustomize/detect_test.go @@ -0,0 +1,369 @@ +package kustomize + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/DataDog/datadog-iac-scanner/pkg/model" + resolverkustomize "github.com/DataDog/datadog-iac-scanner/pkg/resolver/kustomize" + "github.com/stretchr/testify/require" +) + +func TestDetectKindLine_Delegates(t *testing.T) { + ctx := context.Background() + lines := []string{"a: 1", "b: 2"} + f := &model.FileMetadata{ + Kind: model.KindKUSTOMIZE, + FilePath: "x.yaml", + LinesOriginalData: &lines, + LineInfoDocument: map[string]interface{}{"a": map[string]interface{}{"_kics_line": float64(1)}}, + Document: map[string]interface{}{"a": 1}, + } + v := DetectKindLine{}.DetectLine(ctx, f, "a", 1) + require.Equal(t, "x.yaml", v.ResolvedFile) + require.NotEqual(t, -1, v.Line) +} + +func TestTransformerLineForOrigin_prefersPatchPathLine(t *testing.T) { + dir := t.TempDir() + kust := filepath.Join(dir, "kustomization.yaml") + content := "apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\npatches:\n- path: p.yaml\n" + require.NoError(t, os.WriteFile(kust, []byte(content), 0o600)) + patchPath := filepath.Join(dir, "p.yaml") + o := &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginTransformer, + GeneratorConfigFile: kust, + Transformations: []model.KustomizeTransformation{{TransformerPath: patchPath}}, + } + line, p := transformerLineForOrigin(o) + require.Equal(t, kust, p) + require.Equal(t, 4, line, "should point at the list entry referencing the patch file") +} + +func TestDirectSourceDetectLine(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "d.yaml") + require.NoError(t, os.WriteFile(p, []byte("apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: n\nspec:\n replicas: 1\n"), 0o600)) + v := directSourceDetectLine(p, "metadata.name", 1) + require.Equal(t, 4, v.Line) + require.Equal(t, p, v.ResolvedFile) +} + +func TestDirectSourceDetectLine_ASTNestedSequence(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "d.yaml") + require.NoError(t, os.WriteFile(p, []byte("apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: n\nspec:\n template:\n spec:\n containers:\n - name: app\n image: nginx:1.0\n"), 0o600)) + v := directSourceDetectLine(p, "spec.template.spec.containers[0].image", 3) + require.Equal(t, 10, v.Line) + require.Equal(t, "image: nginx:1.0", v.LineWithVulnerability) + require.Equal(t, p, v.ResolvedFile) +} + +func TestTransformerPatchFileLine_nestedContainersUsesAST(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + kust := filepath.Join(dir, "kustomization.yaml") + patchPath := filepath.Join(dir, "patch.yaml") + require.NoError(t, os.WriteFile(kust, []byte("apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\npatches:\n- path: patch.yaml\n"), 0o600)) + patchYAML := `apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + template: + spec: + containers: + - name: sidecar + image: alpine:3 + - name: app + image: nginx:bad +` + require.NoError(t, os.WriteFile(patchPath, []byte(patchYAML), 0o600)) + v := transformerPatchFileLine(ctx, &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginTransformer, + GeneratorConfigFile: kust, + Transformations: []model.KustomizeTransformation{ + {TransformerPath: patchPath}, + }, + }, "spec.template.spec.containers[1].image", 3) + require.Equal(t, 12, v.Line) + require.Equal(t, patchPath, v.ResolvedFile) + require.Contains(t, v.LineWithVulnerability, "nginx:bad") +} + +func TestTransformerPatchFileLine_prefersPatchFile(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + kust := filepath.Join(dir, "kustomization.yaml") + patchPath := filepath.Join(dir, "patch.yaml") + require.NoError(t, os.WriteFile(kust, []byte("apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\npatches:\n- path: patch.yaml\n"), 0o600)) + require.NoError(t, os.WriteFile(patchPath, []byte("apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: app\nspec:\n replicas: 3\n"), 0o600)) + v := transformerPatchFileLine(ctx, &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginTransformer, + GeneratorConfigFile: kust, + Transformations: []model.KustomizeTransformation{ + {TransformerPath: patchPath}, + }, + }, "spec.replicas", 3) + require.Equal(t, 6, v.Line) + require.Equal(t, patchPath, v.ResolvedFile) + require.Equal(t, "replicas: 3", v.LineWithVulnerability) +} + +func TestTransformerPatchFileLine_usesConfiguredInBaseKustomization(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + + baseKust := filepath.Join(base, "kustomization.yaml") + basePatch := filepath.Join(base, "patch.yaml") + require.NoError(t, os.WriteFile(baseKust, []byte("apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\npatches:\n- path: patch.yaml\n"), 0o600)) + require.NoError(t, os.WriteFile(basePatch, []byte("apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: app\nspec:\n replicas: 7\n"), 0o600)) + + v := transformerPatchFileLine(ctx, &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginTransformer, + GeneratorConfigFile: filepath.Join(overlay, "kustomization.yaml"), + Transformations: []model.KustomizeTransformation{ + { + TransformerPath: filepath.Join(overlay, "patch.yaml"), + ConfiguredIn: baseKust, + FieldPath: "patch.yaml", + }, + }, + }, "spec.replicas", 3) + require.Equal(t, 6, v.Line) + require.Equal(t, basePatch, v.ResolvedFile) + require.Equal(t, "replicas: 7", v.LineWithVulnerability) +} + +func TestTransformerPatchFileLine_DoesNotProbeUnrelatedPatchFiles(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + kust := filepath.Join(dir, "kustomization.yaml") + patchA := filepath.Join(dir, "patch-a.yaml") + patchB := filepath.Join(dir, "patch-b.yaml") + require.NoError(t, os.WriteFile(kust, []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +patches: +- path: patch-a.yaml +- path: patch-b.yaml +`), 0o600)) + require.NoError(t, os.WriteFile(patchA, []byte(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + replicas: 3 +`), 0o600)) + require.NoError(t, os.WriteFile(patchB, []byte(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + replicas: 9 +`), 0o600)) + + v := transformerPatchFileLine(ctx, &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginTransformer, + GeneratorConfigFile: kust, + Transformations: []model.KustomizeTransformation{ + {TransformerPath: patchA, FieldPath: "patch-a.yaml"}, + }, + }, "spec.replicas", 3) + require.Equal(t, patchA, v.ResolvedFile) + require.Equal(t, "replicas: 3", v.LineWithVulnerability) +} + +func TestDetectKindLine_GeneratorOrigin(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + kust := filepath.Join(dir, "kustomization.yaml") + content := "apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\nconfigMapGenerator:\n- name: my-map\n literals:\n - k=v\n" + require.NoError(t, os.WriteFile(kust, []byte(content), 0o600)) + + lines := []string{"apiVersion: v1", "kind: ConfigMap", "metadata:", " name: my-map"} + f := &model.FileMetadata{ + Kind: model.KindKUSTOMIZE, + FilePath: filepath.Join(dir, "generated.yaml"), + LinesOriginalData: &lines, + Document: map[string]interface{}{ + "metadata": map[string]interface{}{"name": "my-map"}, + }, + KustomizeOrigin: &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginGenerator, + GeneratorConfigFile: kust, + ResourceName: "my-map", + }, + } + v := DetectKindLine{}.DetectLine(ctx, f, "data.k", 1) + require.Equal(t, kust, v.ResolvedFile) + require.Equal(t, 4, v.Line, "line should point at the list item declaring the generator name") +} + +func TestDetectKindLine_DirectRemoteOrigin_DoesNotTreatURLAsLocalPath(t *testing.T) { + ctx := context.Background() + lines := []string{"apiVersion: apps/v1", "kind: Deployment"} + f := &model.FileMetadata{ + Kind: model.KindKUSTOMIZE, + FilePath: "rendered.yaml", + LinesOriginalData: &lines, + Document: map[string]interface{}{"kind": "Deployment"}, + KustomizeOrigin: &model.KustomizeOrigin{ + OriginKind: model.KustomizeOriginDirect, + SourceFile: "https://github.com/org/repo/base/deployment.yaml", + SourceRepo: "https://github.com/org/repo", + }, + } + v := DetectKindLine{}.DetectLine(ctx, f, "kind", 1) + require.Equal(t, "rendered.yaml", v.ResolvedFile) +} + +func TestDetectKindLine_TransformerBaseOwnedPatchResolvesIntoBasePatchFile(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + + require.NoError(t, os.WriteFile(filepath.Join(base, "deployment.yaml"), []byte(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + replicas: 1 + selector: + matchLabels: + app: app + template: + metadata: + labels: + app: app + spec: + containers: + - name: app + image: nginx +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(base, "patch.yaml"), []byte(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + replicas: 9 +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(base, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- deployment.yaml +patches: +- path: patch.yaml +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +`), 0o600)) + + r := resolverkustomize.NewResolver(resolverkustomize.Options{ + RepoRoot: repo, + AllowHelmInflation: false, + HelmIncludeCRDs: true, + }) + out, err := r.Resolve(ctx, overlay) + require.NoError(t, err) + require.Empty(t, out.Diagnostics, "%+v", out.Diagnostics) + require.Len(t, out.File, 1) + require.NotNil(t, out.File[0].Origin) + + lines := strings.Split(string(out.File[0].Content), "\n") + f := &model.FileMetadata{ + Kind: model.KindKUSTOMIZE, + FilePath: out.File[0].FileName, + LinesOriginalData: &lines, + Document: map[string]interface{}{ + "spec": map[string]interface{}{ + "replicas": 9, + }, + }, + KustomizeOrigin: out.File[0].Origin, + } + v := DetectKindLine{}.DetectLine(ctx, f, "spec.replicas", 1) + require.Equal(t, filepath.Join(base, "patch.yaml"), v.ResolvedFile) + require.Equal(t, 6, v.Line) + require.Equal(t, "replicas: 9", v.LineWithVulnerability) +} + +func TestDetectKindLine_TransformerFallsBackToOriginalSourceForUntouchedField(t *testing.T) { + ctx := context.Background() + repo := t.TempDir() + base := filepath.Join(repo, "base") + overlay := filepath.Join(repo, "overlay") + require.NoError(t, os.MkdirAll(base, 0o700)) + require.NoError(t, os.MkdirAll(overlay, 0o700)) + + baseDeployment := filepath.Join(base, "deployment.yaml") + require.NoError(t, os.WriteFile(baseDeployment, []byte(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + replicas: 3 + selector: + matchLabels: + app: app + template: + metadata: + labels: + app: app + spec: + containers: + - name: app + image: nginx +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(base, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- deployment.yaml +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(overlay, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- ../base +namespace: prod +`), 0o600)) + + r := resolverkustomize.NewResolver(resolverkustomize.Options{ + RepoRoot: repo, + AllowHelmInflation: false, + HelmIncludeCRDs: true, + }) + out, err := r.Resolve(ctx, overlay) + require.NoError(t, err) + require.Empty(t, out.Diagnostics, "%+v", out.Diagnostics) + require.Len(t, out.File, 1) + require.NotNil(t, out.File[0].Origin) + require.Equal(t, model.KustomizeOriginTransformer, out.File[0].Origin.OriginKind) + + lines := strings.Split(string(out.File[0].Content), "\n") + f := &model.FileMetadata{ + Kind: model.KindKUSTOMIZE, + FilePath: out.File[0].FileName, + LinesOriginalData: &lines, + Document: map[string]interface{}{ + "spec": map[string]interface{}{ + "replicas": 3, + }, + }, + KustomizeOrigin: out.File[0].Origin, + } + v := DetectKindLine{}.DetectLine(ctx, f, "spec.replicas", 1) + require.Equal(t, baseDeployment, v.ResolvedFile) + require.Equal(t, 6, v.Line) + require.Equal(t, "replicas: 3", v.LineWithVulnerability) +} diff --git a/pkg/detector/kustomize/direct_line.go b/pkg/detector/kustomize/direct_line.go new file mode 100644 index 000000000..ee7f90386 --- /dev/null +++ b/pkg/detector/kustomize/direct_line.go @@ -0,0 +1,211 @@ +package kustomize + +import ( + "bytes" + "path/filepath" + "strconv" + "strings" + + "github.com/DataDog/datadog-iac-scanner/pkg/detector" + "github.com/DataDog/datadog-iac-scanner/pkg/model" + "github.com/DataDog/datadog-iac-scanner/pkg/rootfile" + yamlv3 "gopkg.in/yaml.v3" +) + +// directSourceDetectLine maps a finding to a plain resources: file when render diverges from source YAML. +// Tries AST walk along searchKey, then a simple key-tail line match. +func directSourceDetectLine(srcPath, searchKey string, outputLines int) model.VulnerabilityLines { + srcPath = filepath.Clean(srcPath) + data, err := rootfile.ReadFile(srcPath) + if err != nil { + return model.VulnerabilityLines{Line: -1} + } + norm := strings.ReplaceAll(string(data), "\r\n", "\n") + lines := strings.Split(norm, "\n") + + if ln := astLineForSearchKey([]byte(norm), searchKey); ln > 0 && ln <= len(lines) { + trim := strings.TrimSpace(lines[ln-1]) + return model.VulnerabilityLines{ + Line: ln, + VulnLines: detector.GetAdjacentVulnLines(ln-1, outputLines, lines), + LineWithVulnerability: trim, + ResolvedFile: srcPath, + VulnerablilityLocation: model.ResourceLocation{ + Start: model.ResourceLine{Line: ln, Col: 1}, + End: model.ResourceLine{Line: ln, Col: 1}, + }, + } + } + + tail := searchKey + if i := strings.LastIndex(searchKey, "."); i >= 0 { + tail = searchKey[i+1:] + } + tail = strings.TrimSpace(tail) + if idx := strings.Index(tail, "["); idx >= 0 { + tail = tail[:idx] + } + if tail == "" { + return model.VulnerabilityLines{Line: -1} + } + for i, line := range lines { + trim := strings.TrimSpace(strings.TrimRight(line, "\r")) + if strings.HasPrefix(trim, tail+":") || strings.HasPrefix(trim, tail+" :") { + return model.VulnerabilityLines{ + Line: i + 1, + VulnLines: detector.GetAdjacentVulnLines(i, outputLines, lines), + LineWithVulnerability: trim, + ResolvedFile: srcPath, + VulnerablilityLocation: model.ResourceLocation{ + Start: model.ResourceLine{Line: i + 1, Col: 1}, + End: model.ResourceLine{Line: i + 1, Col: 1}, + }, + } + } + } + return model.VulnerabilityLines{Line: -1} +} + +// astLineForSearchKey returns the 1-based line for searchKey in multi-doc YAML (dotted path with optional [i]); 0 if none. +func astLineForSearchKey(src []byte, searchKey string) int { + if len(src) == 0 || strings.TrimSpace(searchKey) == "" { + return 0 + } + parts := splitSearchKeyPath(searchKey) + if len(parts) == 0 { + return 0 + } + dec := yamlv3.NewDecoder(bytes.NewReader(src)) + for { + var doc yamlv3.Node + if err := dec.Decode(&doc); err != nil { + break + } + if ln := walkYAMLPath(&doc, parts); ln > 0 { + return ln + } + } + return 0 +} + +type yamlPathPart struct { + key string + index *int +} + +const yamlPathPartInitialCap = 8 + +func splitSearchKeyPath(searchKey string) []yamlPathPart { + out := make([]yamlPathPart, 0, yamlPathPartInitialCap) + for _, raw := range strings.Split(searchKey, ".") { + part := strings.TrimSpace(raw) + if part == "" { + continue + } + p := yamlPathPart{key: part} + if idx := strings.Index(part, "["); idx >= 0 { + p.key = part[:idx] + if end := strings.Index(part[idx:], "]"); end > 1 { + if n, err := strconv.Atoi(part[idx+1 : idx+end]); err == nil { + p.index = &n + } + } + } + out = append(out, p) + } + return out +} + +func walkYAMLPath(node *yamlv3.Node, parts []yamlPathPart) int { + if node == nil || len(parts) == 0 { + return 0 + } + cur := unwrapDocumentNode(node) + var matchLine int + for _, p := range parts { + if cur == nil || p.key == "" { + return 0 + } + switch cur.Kind { + case yamlv3.MappingNode: + next := lookupMappingValue(cur, p.key) + if next == nil { + return matchLine + } + matchLine = next.Line + cur = unwrapDocumentNode(next) + if p.index != nil { + cur = lookupSequenceIndex(cur, *p.index) + if cur == nil { + return matchLine + } + matchLine = cur.Line + } + case yamlv3.SequenceNode: + cur = lookupSequenceElement(cur, p.key, p.index) + if cur == nil { + return matchLine + } + matchLine = cur.Line + default: + return matchLine + } + } + return matchLine +} + +func unwrapDocumentNode(n *yamlv3.Node) *yamlv3.Node { + if n == nil { + return nil + } + if n.Kind == yamlv3.DocumentNode && len(n.Content) > 0 { + return n.Content[0] + } + return n +} + +func lookupMappingValue(node *yamlv3.Node, key string) *yamlv3.Node { + if node == nil || node.Kind != yamlv3.MappingNode { + return nil + } + for i := 0; i+1 < len(node.Content); i += 2 { + if node.Content[i].Value == key { + return node.Content[i+1] + } + } + return nil +} + +func lookupSequenceIndex(node *yamlv3.Node, idx int) *yamlv3.Node { + if node == nil || node.Kind != yamlv3.SequenceNode || idx < 0 || idx >= len(node.Content) { + return nil + } + return node.Content[idx] +} + +func lookupSequenceElement(node *yamlv3.Node, key string, idx *int) *yamlv3.Node { + if node == nil || node.Kind != yamlv3.SequenceNode { + return nil + } + if idx != nil { + if elem := lookupSequenceIndex(node, *idx); elem != nil { + if mapped := lookupMappingValue(unwrapDocumentNode(elem), key); mapped != nil { + return mapped + } + return elem + } + } + for _, elem := range node.Content { + elem = unwrapDocumentNode(elem) + if elem == nil { + continue + } + if mapped := lookupMappingValue(elem, key); mapped != nil { + return mapped + } + if nameNode := lookupMappingValue(elem, "name"); nameNode != nil && nameNode.Value == key { + return elem + } + } + return nil +} diff --git a/pkg/detector/kustomize/generator_line.go b/pkg/detector/kustomize/generator_line.go new file mode 100644 index 000000000..c26b79f11 --- /dev/null +++ b/pkg/detector/kustomize/generator_line.go @@ -0,0 +1,312 @@ +package kustomize + +import ( + "bufio" + "bytes" + "context" + "path/filepath" + "strings" + + "github.com/DataDog/datadog-iac-scanner/pkg/detector" + "github.com/DataDog/datadog-iac-scanner/pkg/model" + "github.com/DataDog/datadog-iac-scanner/pkg/rootfile" + zlog "github.com/rs/zerolog/log" + "gopkg.in/yaml.v3" +) + +const ( + yamlKeyConfigMapGenerator = "configMapGenerator:" + yamlKeySecretGenerator = "secretGenerator:" +) + +func isGeneratorSectionHeader(trimmed string) bool { + return trimmed == yamlKeyConfigMapGenerator || trimmed == yamlKeySecretGenerator +} + +// generatorConfigLine returns the best 1-based line for a generator-produced resource in the kustomization file. +func generatorConfigLine(origin *model.KustomizeOrigin, resourceName string) (line1 int, path string) { + if origin == nil || origin.GeneratorConfigFile == "" || resourceName == "" { + return 1, "" + } + p := filepath.Clean(origin.GeneratorConfigFile) + data, err := rootfile.ReadFile(p) + if err != nil { + return 1, p + } + lines := strings.Split(string(data), "\n") + if ln, pth, ok := generatorLineFromScan(lines, resourceName, p); ok { + return ln, pth + } + return generatorLineFallback(lines, resourceName, p) +} + +func generatorLineFromScan(lines []string, resourceName, p string) (line1 int, path string, ok bool) { + inGen := false + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if !inGen { + if isGeneratorSectionHeader(trimmed) { + inGen = true + } + continue + } + if line != "" && line[0] != ' ' && line[0] != '\t' { + if strings.HasPrefix(trimmed, "#") { + continue + } + if isGeneratorSectionHeader(trimmed) { + inGen = true + continue + } + inGen = false + continue + } + item := strings.TrimSpace(strings.TrimPrefix(trimmed, "-")) + if strings.HasPrefix(item, "name:") { + rest := strings.TrimSpace(strings.TrimPrefix(item, "name:")) + rest = strings.Trim(rest, `"'`) + for _, candidate := range generatorNameCandidates(resourceName) { + if rest == candidate { + return i + 1, p, true + } + } + } + } + return 0, "", false +} + +func generatorLineFallback(lines []string, resourceName, p string) (line1 int, path string) { + for i, line := range lines { + for _, candidate := range generatorNameCandidates(resourceName) { + if strings.Contains(line, "name:") && strings.Contains(line, candidate) { + return i + 1, p + } + } + } + return 1, p +} + +// transformerConfigLine returns a 1-based line in the kustomization for transformer-related config. +func transformerConfigLine(origin *model.KustomizeOrigin) (line1 int, path string) { + if origin == nil || origin.GeneratorConfigFile == "" { + return 1, "" + } + p := filepath.Clean(origin.GeneratorConfigFile) + data, err := rootfile.ReadFile(p) + if err != nil { + return 1, p + } + sc := bufio.NewScanner(bytes.NewReader(data)) + keys := []string{"transformers:", "patches:", "patchesStrategicMerge:", "patchesJson6902:"} + for lineNo := 1; sc.Scan(); lineNo++ { + t := strings.TrimSpace(sc.Text()) + for _, k := range keys { + if strings.HasPrefix(t, k) { + return lineNo, p + } + } + } + return 1, p +} + +// transformerPatchReferenceLine is the 1-based line in the kustomization that cites the patch path. +func transformerPatchReferenceLine(origin *model.KustomizeOrigin) (line1 int, path string) { + if origin == nil || origin.GeneratorConfigFile == "" || len(origin.Transformations) == 0 { + return 0, "" + } + for _, tr := range origin.Transformations { + kustPath := transformerDeclaredConfigPath(origin, tr) + kustDir := filepath.Dir(kustPath) + data, err := rootfile.ReadFile(kustPath) + if err != nil { + continue + } + lines := strings.Split(string(data), "\n") + for _, candidate := range transformerPatchCandidatePaths(origin, tr) { + rel, err := filepath.Rel(kustDir, filepath.Clean(candidate)) + if err != nil { + continue + } + rel = filepath.ToSlash(rel) + variants := []string{rel} + if !strings.HasPrefix(rel, "./") { + variants = append(variants, "./"+rel) + } + for lineNo, line := range lines { + t := strings.TrimSpace(line) + for _, want := range variants { + if !strings.Contains(t, want) { + continue + } + if strings.Contains(t, "path:") || strings.HasPrefix(t, "-") { + return lineNo + 1, kustPath + } + } + } + } + } + return 0, "" +} + +func transformerLineForOrigin(origin *model.KustomizeOrigin) (line1 int, path string) { + if origin == nil { + return 1, "" + } + if ln, p := transformerPatchReferenceLine(origin); ln > 0 { + return ln, p + } + return transformerConfigLine(origin) +} + +func transformerDeclaredConfigPath(origin *model.KustomizeOrigin, tr model.KustomizeTransformation) string { + if tr.ConfiguredIn != "" { + return filepath.Clean(tr.ConfiguredIn) + } + if origin == nil { + return "" + } + return filepath.Clean(origin.GeneratorConfigFile) +} + +func transformerPatchCandidatePaths(origin *model.KustomizeOrigin, tr model.KustomizeTransformation) []string { + seen := make(map[string]struct{}) + var out []string + add := func(p string) { + p = filepath.Clean(p) + if p == "" { + return + } + if _, ok := seen[p]; ok { + return + } + seen[p] = struct{}{} + out = append(out, p) + } + + kustPath := transformerDeclaredConfigPath(origin, tr) + kustDir := filepath.Dir(kustPath) + declared := patchPathsDeclaredInKustomization(kustPath) + declaredAbs := make(map[string]struct{}, len(declared)) + for _, rel := range declared { + if kustDir == "" { + continue + } + declaredAbs[filepath.Clean(filepath.Join(kustDir, rel))] = struct{}{} + } + if tr.FieldPath != "" && kustDir != "" { + add(filepath.Join(kustDir, tr.FieldPath)) + } + if tr.TransformerPath != "" { + cleanTransformerPath := filepath.Clean(tr.TransformerPath) + if len(declaredAbs) == 0 { + if tr.FieldPath != "" || cleanTransformerPath != filepath.Clean(kustDir) { + add(cleanTransformerPath) + } + } else if _, ok := declaredAbs[cleanTransformerPath]; ok { + add(cleanTransformerPath) + } + } + if len(out) == 0 { + // Single declared patch only: multiple patches would make this fallback ambiguous. + if len(declared) == 1 && kustDir != "" { + add(filepath.Join(kustDir, declared[0])) + } + } + return out +} + +func appendPatchPathsMixedList(list []interface{}, add func(string)) { + for _, item := range list { + switch x := item.(type) { + case string: + add(x) + case map[string]interface{}: + if p, ok := x["path"].(string); ok { + add(p) + } + } + } +} + +func patchPathsDeclaredInKustomization(kustPath string) []string { + clean := filepath.Clean(kustPath) + data, err := rootfile.ReadFile(clean) + if err != nil { + return nil + } + var doc map[string]interface{} + if err := yaml.Unmarshal(data, &doc); err != nil { + return nil + } + var out []string + add := func(p string) { + p = strings.TrimSpace(p) + if p == "" || strings.Contains(p, "://") { + return + } + out = append(out, p) + } + + if list, ok := doc["patches"].([]interface{}); ok { + appendPatchPathsMixedList(list, add) + } + if list, ok := doc["patchesStrategicMerge"].([]interface{}); ok { + appendPatchPathsMixedList(list, add) + } + if list, ok := doc["patchesJson6902"].([]interface{}); ok { + for _, item := range list { + if m, ok := item.(map[string]interface{}); ok { + if p, ok := m["path"].(string); ok { + add(p) + } + } + } + } + return out +} + +// transformerPatchFileLine maps a finding into the transformer patch file; returns Line 0 when not mappable. +func transformerPatchFileLine( + ctx context.Context, + origin *model.KustomizeOrigin, + searchKey string, + outputLines int, +) model.VulnerabilityLines { + if origin == nil || len(origin.Transformations) == 0 || strings.TrimSpace(searchKey) == "" { + return model.VulnerabilityLines{} + } + for i := len(origin.Transformations) - 1; i >= 0; i-- { + tr := origin.Transformations[i] + for _, p := range transformerPatchCandidatePaths(origin, tr) { + info, err := rootfile.Lstat(p) + if err != nil { + zlog.Ctx(ctx).Debug().Err(err).Str("transformer_patch", p).Msg("kustomize transformer patch path not usable") + continue + } + if info.IsDir() { + continue + } + data, err := rootfile.ReadFile(p) + if err != nil { + continue + } + if ln := astLineForSearchKey(data, searchKey); ln > 0 { + fileLines := strings.Split(strings.ReplaceAll(string(data), "\r\n", "\n"), "\n") + if ln <= len(fileLines) { + t := strings.TrimSpace(strings.TrimRight(fileLines[ln-1], "\r")) + return model.VulnerabilityLines{ + Line: ln, + VulnLines: detector.GetAdjacentVulnLines(ln-1, outputLines, fileLines), + LineWithVulnerability: t, + ResolvedFile: p, + VulnerablilityLocation: model.ResourceLocation{ + Start: model.ResourceLine{Line: ln, Col: 1}, + End: model.ResourceLine{Line: ln, Col: 1}, + }, + } + } + } + } + } + return model.VulnerabilityLines{} +} diff --git a/pkg/detector/kustomize/generator_line_test.go b/pkg/detector/kustomize/generator_line_test.go new file mode 100644 index 000000000..b75181ac8 --- /dev/null +++ b/pkg/detector/kustomize/generator_line_test.go @@ -0,0 +1,35 @@ +package kustomize + +import ( + "os" + "path/filepath" + "testing" + + "github.com/DataDog/datadog-iac-scanner/pkg/model" + "github.com/stretchr/testify/require" +) + +func TestGeneratorConfigLine_secondGeneratorBlock(t *testing.T) { + dir := t.TempDir() + kust := filepath.Join(dir, "kustomization.yaml") + content := `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +configMapGenerator: +- name: first + literals: + - a=b +secretGenerator: +- name: secret-a + literals: + - s=t +configMapGenerator: +- name: second + literals: + - c=d +` + require.NoError(t, os.WriteFile(kust, []byte(content), 0o600)) + o := &model.KustomizeOrigin{GeneratorConfigFile: kust} + line, p := generatorConfigLine(o, "second") + require.Equal(t, kust, p) + require.Equal(t, 12, line) +} diff --git a/pkg/detector/kustomize/helper_main_test.go b/pkg/detector/kustomize/helper_main_test.go new file mode 100644 index 000000000..c0e105b10 --- /dev/null +++ b/pkg/detector/kustomize/helper_main_test.go @@ -0,0 +1,14 @@ +package kustomize + +import ( + "os" + "testing" + + resolverkustomize "github.com/DataDog/datadog-iac-scanner/pkg/resolver/kustomize" +) + +// TestMain ensures kustomize subprocess render (re-exec of this test binary) works during detector tests. +func TestMain(m *testing.M) { + resolverkustomize.MaybeRunAsKustomizeRenderHelper() + os.Exit(m.Run()) +} diff --git a/pkg/detector/kustomize/namehash.go b/pkg/detector/kustomize/namehash.go new file mode 100644 index 000000000..58deb0edb --- /dev/null +++ b/pkg/detector/kustomize/namehash.go @@ -0,0 +1,21 @@ +package kustomize + +import "regexp" + +// Default generator name suffix is a 10-char hash; strip it when matching kustomization names. +var kustomizeNameHashSuffix = regexp.MustCompile(`-[a-z0-9]{10}$`) + +func stripKustomizeNameHashSuffix(name string) string { + return kustomizeNameHashSuffix.ReplaceAllString(name, "") +} + +func generatorNameCandidates(resourceName string) []string { + if resourceName == "" { + return nil + } + out := []string{resourceName} + if s := stripKustomizeNameHashSuffix(resourceName); s != resourceName && s != "" { + out = append(out, s) + } + return out +} diff --git a/pkg/detector/kustomize/namehash_test.go b/pkg/detector/kustomize/namehash_test.go new file mode 100644 index 000000000..a3d326be2 --- /dev/null +++ b/pkg/detector/kustomize/namehash_test.go @@ -0,0 +1,27 @@ +package kustomize + +import ( + "os" + "path/filepath" + "testing" + + "github.com/DataDog/datadog-iac-scanner/pkg/model" + "github.com/stretchr/testify/require" +) + +func TestStripKustomizeNameHashSuffix(t *testing.T) { + require.Equal(t, "my-map", stripKustomizeNameHashSuffix("my-map-h4kk2gt9cm")) + require.Equal(t, "x", stripKustomizeNameHashSuffix("x")) +} + +func TestGeneratorConfigLine_matchesWithHashedLookupName(t *testing.T) { + // kustomization declares "my-map"; detector may still receive a hashed name from document metadata. + content := "apiVersion: kustomize.config.k8s.io/v1beta1\nkind: Kustomization\nconfigMapGenerator:\n- name: my-map\n literals:\n - k=v\n" + dir := t.TempDir() + p := filepath.Join(dir, "kustomization.yaml") + require.NoError(t, os.WriteFile(p, []byte(content), 0o600)) + o := &model.KustomizeOrigin{GeneratorConfigFile: p} + line, gotPath := generatorConfigLine(o, "my-map-h4kk2gt9cm") + require.Equal(t, p, gotPath) + require.Equal(t, 4, line) +} From f8854e98501cb788e50501e2ed03162694b65d82 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Wed, 29 Apr 2026 13:51:57 +0200 Subject: [PATCH 16/17] Wire the Kustomize detector into engine line attribution and skip the search-line fast path for kustomize files. --- pkg/engine/inspector.go | 2 ++ pkg/engine/vulnerability_builder.go | 13 ++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/engine/inspector.go b/pkg/engine/inspector.go index a4de52a50..724fffb62 100644 --- a/pkg/engine/inspector.go +++ b/pkg/engine/inspector.go @@ -18,6 +18,7 @@ import ( "github.com/DataDog/datadog-iac-scanner/pkg/detector" "github.com/DataDog/datadog-iac-scanner/pkg/detector/docker" "github.com/DataDog/datadog-iac-scanner/pkg/detector/helm" + "github.com/DataDog/datadog-iac-scanner/pkg/detector/kustomize" "github.com/DataDog/datadog-iac-scanner/pkg/detector/terraform" "github.com/DataDog/datadog-iac-scanner/pkg/engine/source" "github.com/DataDog/datadog-iac-scanner/pkg/featureflags" @@ -158,6 +159,7 @@ func NewInspector( lineDetector := detector.NewDetectLine(tracker.GetOutputLines()). Add(helm.DetectKindLine{}, model.KindHELM). + Add(kustomize.DetectKindLine{}, model.KindKUSTOMIZE). Add(docker.DetectKindLine{}, model.KindDOCKER). Add(terraform.DetectKindLine{}, model.KindTerraform) diff --git a/pkg/engine/vulnerability_builder.go b/pkg/engine/vulnerability_builder.go index a57e89386..4cd3fab45 100644 --- a/pkg/engine/vulnerability_builder.go +++ b/pkg/engine/vulnerability_builder.go @@ -62,6 +62,16 @@ func modifyVulSearchKeyReference(doc interface{}, originalSearchKey string, stri return originalSearchKey, false } +func useSearchLineFastPathFor(file *model.FileMetadata) bool { + if slices.Contains([]model.FileKind{model.KindHELM, model.KindTerraform}, file.Kind) { + return false + } + if file.Kind == model.KindKUSTOMIZE { + return false + } + return len(file.ResolvedFiles) == 0 +} + // DefaultVulnerabilityBuilder defines a vulnerability builder to execute default actions of scan var DefaultVulnerabilityBuilder = func(ctx context.Context, qCtx *QueryContext, tracker Tracker, @@ -141,7 +151,8 @@ var DefaultVulnerabilityBuilder = func(ctx context.Context, qCtx *QueryContext, // terraform detection tries to capture blocks whenever possible which is contradictory with the searchLine // only .tf files use this detection, therefore, terraform plans and cdk (json) are not excluded - if !slices.Contains([]model.FileKind{model.KindHELM, model.KindTerraform}, file.Kind) && len(file.ResolvedFiles) == 0 { + useSearchLineFastPath := useSearchLineFastPathFor(file) + if useSearchLineFastPath { searchLineCalc := &searchLineCalculator{ lineNr: -1, vObj: vObj, From d349b4e34d33d5a6ecf526120a312111b7ffb93a Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Wed, 29 Apr 2026 13:52:03 +0200 Subject: [PATCH 17/17] Add service-level integration tests for Kustomize line attribution. --- pkg/kics/service_test.go | 126 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/pkg/kics/service_test.go b/pkg/kics/service_test.go index 39230d1bc..6c7f490c5 100644 --- a/pkg/kics/service_test.go +++ b/pkg/kics/service_test.go @@ -8,6 +8,8 @@ package kics import ( "context" "fmt" + "os" + "path/filepath" "reflect" "sync" "testing" @@ -22,6 +24,8 @@ import ( terraformParser "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform" yamlParser "github.com/DataDog/datadog-iac-scanner/pkg/parser/yaml/default" "github.com/DataDog/datadog-iac-scanner/pkg/resolver" + kustomizeResolver "github.com/DataDog/datadog-iac-scanner/pkg/resolver/kustomize" + "github.com/stretchr/testify/require" ) // TestService tests the functions [GetVulnerabilities(), StartScan()] and all the methods called by them @@ -77,6 +81,7 @@ func TestService(t *testing.T) { //nolint } for _, tt := range tests { s := make([]*Service, 0, len(tt.fields.Parser)) + resolverDiagnostics := NewResolverDiagnosticsState() for _, parser := range tt.fields.Parser { s = append(s, &Service{ SourceProvider: tt.fields.SourceProvider, @@ -85,6 +90,7 @@ func TestService(t *testing.T) { //nolint Inspector: tt.fields.Inspector, Tracker: tt.fields.Tracker, Resolver: tt.fields.Resolver, + ResolverDiagnostics: resolverDiagnostics, }) } t.Run(fmt.Sprintf("%s", tt.name+"_get_vulnerabilities"), func(t *testing.T) { @@ -139,3 +145,123 @@ func createParserSourceProvider(path string) ([]*parser.Parser, return mockParser, mockFilesSource, mockResolver } + +func TestSaveResolverDiagnostics_DedupesAcrossServicesInSameScan(t *testing.T) { + store := storage.NewMemoryStorage() + state := NewResolverDiagnosticsState() + scanID := "scanID" + diag := model.ResolverDiagnostic{ + FilePath: "/tmp/overlay/kustomization.yaml", + Message: "render failed", + QueryID: "kustomize-render-failed", + Line: 0, + } + + serviceA := &Service{ + Storage: store, + ResolverDiagnostics: state, + } + serviceB := &Service{ + Storage: store, + ResolverDiagnostics: state, + } + + require.NoError(t, serviceA.saveResolverDiagnostics(context.Background(), scanID, []model.ResolverDiagnostic{diag})) + require.NoError(t, serviceB.saveResolverDiagnostics(context.Background(), scanID, []model.ResolverDiagnostic{diag})) + + vulns, err := store.GetVulnerabilities(context.Background(), scanID) + require.NoError(t, err) + require.Len(t, vulns, 1) + require.Equal(t, "", vulns[0].Platform) + require.Equal(t, 1, vulns[0].Line) +} + +// Regression: a single unparseable rendered doc (e.g. Kustomize generator +// output with a virtual filename) must not drop resFiles.Excluded; otherwise +// the walker re-scans patches / base files as raw YAML and produces duplicate +// or partial-document findings. +func TestResolverSink_PropagatesExcludedOnParseFailure(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- pod.yaml +configMapGenerator: +- name: app-config + literals: + - KEY=value +`), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(root, "pod.yaml"), []byte(`apiVersion: v1 +kind: Pod +metadata: + name: demo + namespace: default +spec: + containers: + - name: app + image: nginx:1.21 + envFrom: + - configMapRef: + name: app-config +`), 0o600)) + + resolverWithKustomize, err := resolver.NewBuilder(). + Add(ctx, kustomizeResolver.NewResolver(kustomizeResolver.Options{RepoRoot: root})). + Build(ctx) + require.NoError(t, err) + + kicsParser, err := parser.NewBuilder(ctx). + Add(&yamlParser.Parser{}). + Build([]string{"Kubernetes"}, []string{""}) + require.NoError(t, err) + + tr, err := tracker.NewTracker(1) + require.NoError(t, err) + + service := &Service{ + Storage: storage.NewMemoryStorage(), + ResolverDiagnostics: NewResolverDiagnosticsState(), + Parser: kicsParser[0], + Resolver: resolverWithKustomize, + Tracker: tr, + } + + excluded, err := service.resolverSink(ctx, root, "scanID", false, 0) + require.NoError(t, err) + + // Generator emits a virtual generated-ConfigMap-*.yaml (no file on disk) + // which fails Parser.Parse; excludes must still surface the real sources. + require.Contains(t, excluded, filepath.Join(root, "kustomization.yaml")) + require.Contains(t, excluded, filepath.Join(root, "pod.yaml")) +} + +func TestSaveResolverDiagnostics_UsesServicePlatform(t *testing.T) { + store := storage.NewMemoryStorage() + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "kustomization.yaml"), []byte(`apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: [] +`), 0o600)) + resolverWithKustomize, err := resolver.NewBuilder(). + Add(context.Background(), kustomizeResolver.NewResolver(kustomizeResolver.Options{RepoRoot: root})). + Build(context.Background()) + require.NoError(t, err) + service := &Service{ + Storage: store, + ResolverDiagnostics: NewResolverDiagnosticsState(), + Parser: &parser.Parser{Platform: []string{"Knative", "Kubernetes"}}, + Resolver: resolverWithKustomize, + } + + require.NoError(t, service.saveResolverDiagnostics(context.Background(), "scanID", []model.ResolverDiagnostic{{ + FilePath: filepath.Join(root, "kustomization.yaml"), + Message: "render failed", + QueryID: "kustomize-render-failed", + }})) + + vulns, err := store.GetVulnerabilities(context.Background(), "scanID") + require.NoError(t, err) + require.Len(t, vulns, 1) + require.Equal(t, "Kubernetes", vulns[0].Platform) +}