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)
This PR intends to move the loader api to
internal. Only the necessary methods which
are needed for the api have been put into
`pkg/loader.go`.
Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
This PR is an effort towards internalizing public APIs.
It moves some of the builtinconstants to internal/
Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
* Add accumulateResources error tests for local files.
Add tests demonstrating accumulateResources errors when the resource is
a local file. Works to address #4807.
* Improve readability
* Move api/filesys to kyaml/filesys
* Add deprecated version of api/filesys with aliases to new code
* Use new kyaml/filesys package and update dependencies
* Migrate to kyaml/filesys and update dependencies
* Skip tests that break on Windows
On MacOS /var is a symlink for /private/var and we can end up having the
loader having a /private/var path while the TMPDIR has a /var path which
triggers a panic.
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
* Make fsNode handle correctly consecutive reads and writes
* Check for directories in ReadFile and add some error checks
* Update comment
* Improved docs and added better test
* Move test into its own file protected by built constraint
* Use manual test since iotest.TestReader is only available in Go 1.16
The apimachinery code path, in its final marshalling
for output, calls Marshall
https://github.com/go-yaml/yaml/blob/v2/yaml.go#L199
This code path (via apimachinery Unstructured types)
has no JSON schema tags
https://yaml.org/spec/1.2/spec.html#id2803311
so it adds quotes to values that smell like
booleans and ints (e.g. `false` becomes `"false"`).
The kyaml code path, OTOH, uses such tags,
so generally does not quote ints and booleans.
This PR isolates this difference in behavior to
one set of tests (using data fields in configmaps
in api/krusty/configmaps_test.go) so that
they don't confuse other tests that cover
completely different behaviors.
This PR
- defines a patch conflict detector interface,
- extracts implementations of the interface from the
merginator code, making the merginator code
independent of --enable_kyaml.
- injects those implementations into kustomize
as a function of --enable_kyaml.
So, instead of using different merginators to combine
resmaps, this pr allows the use of a single patch merge
code path that uses different conflict detectors.
So instead of debating how to merge, we're now only
considering whether to warn on conflict detection
in one transformer.
This PR is in service of #3304, eliminating seven
instances where --enable_kyaml was consulted. These
were cases where conflict detection wasn't an issue
(but merging patches was).
