mirror of
https://github.com/kubernetes-sigs/kustomize.git
synced 2026-09-17 04:51:31 +00:00
65279bb0c2
Motivation: HelmChart.AsHelmArgs() appends ReleaseName as the first bare positional argument to `helm template`, and pullCommand() appends Name as a bare positional argument to `helm pull` (when a repo is set and the chart isn't already cached locally). Neither value is preceded by a `--` delimiter before being handed to exec.Command. Helm's own flag parser does not distinguish a bare positional argument from a flag: if a kustomization.yaml sets, for example, releaseName: --post-renderer=./evil.sh, helm interprets that as a --post-renderer flag rather than a release name, and executes the attacker-supplied script during `kustomize build --enable-helm` (or `kubectl kustomize --enable-helm`). This is a real, demonstrated flag-injection path reachable from an untrusted kustomization.yaml plus --enable-helm; it is not a claim about every possible helm argument, only the two fields that are passed as bare positionals. Other HelmChart fields (Namespace, ValuesFile, KubeVersion, etc.) are passed as `--flag value` pairs, where helm's pflag-based parser consumes the very next token as the flag's value regardless of its content, so they are not exploitable the same way and are out of scope for this change. Approach: Reject a releaseName or name that starts with '-' in validateArgs(), which runs during Config() before any helm subprocess is spawned. The check is added to the plugin source (plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go) and mirrored into the generated copy (api/internal/builtins/HelmChartInflationGenerator.go) via `go generate .` (pluginator), matching how this plugin is normally maintained. A small test harness helper, ErrorFromLoadAndRunGenerator, was added to api/testutils/kusttest/harnessenhanced.go, modeled on the existing ErrorFromLoadAndRunTransformer helper, so the new tests can assert on the Config()-time validation error without needing an actual helm binary installed. Validation: - `cd api && go build ./... && go vet ./...` pass. - `cd plugin/builtin/helmchartinflationgenerator && go vet ./...` passes. (`go build ./...` in that directory fails with "function main is undeclared" both before and after this change; it's a //go:generate pluginator source file compiled specially, not a standalone main package, so plain `go build` there is not meaningful.) - Added TestHelmChartInflationGeneratorRejectsFlagLikeReleaseName and TestHelmChartInflationGeneratorRejectsFlagLikeChartName in plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator_test.go. Verified both fail-then-pass: with each new HasPrefix check temporarily removed, `go test ./... -run TestHelmChartInflationGeneratorRejectsFlagLike... -v` fails with an "unable to run: helmV3 ... executable file not found" error, proving execution reaches the real helm subprocess call with the injected flag; restoring the check makes the same test pass with the expected "must not start with '-'" error, confirming validation now happens before any subprocess is spawned. - `go test ./types/... ./testutils/... ./internal/builtins/...` in api/ pass. `go test ./krusty/...` has one unrelated pre-existing failure, TestAddManagedbyLabel, which fails identically on unmodified master: it expects a version string baked in via -ldflags during `make test` that plain `go test` does not set. - golangci-lint v1.64.8 (the version pinned in hack/go.mod, matching what CI's `make lint` installs) run against the changed packages is clean. Report: https://github.com/kubernetes-sigs/kustomize/issues/6241 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5 (via Claude Code)