Skip to content

Reject helm template name and namespace starting with '-' - #1886

Open
himsngh wants to merge 1 commit into
carvel-dev:developfrom
himsngh:himsngh/fix-helm-template-name-flag-injection
Open

himsngh wants to merge 1 commit into
carvel-dev:developfrom
himsngh:himsngh/fix-helm-template-name-flag-injection

Conversation

@himsngh

@himsngh himsngh commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

spec.template[].helmTemplate.name is passed to "helm template" as the first positional argument without validation. A value starting with '-' is parsed by helm as a flag instead of a release name, so arbitrary helm template flags can be supplied through this field, e.g. "--set-file=key=" makes helm read a file from the controller (or sidecarexec) container filesystem into the rendered manifests. The same value can also be set on a PackageInstall through the ext.packaging.carvel.dev/helm-template-name annotation.

Return an error before invoking helm when the resolved name or namespace starts with '-'. Both values come from the App spec (or the PackageInstall annotations), so they are validated at the point where the helm arguments are built, which covers both paths. Valid release names and namespaces can never start with '-', so this does not change behavior for existing valid configurations.

Changes:

  1. pkg/template/helm_template.go

    • Reject name and namespace values starting with '-'
  2. pkg/template/helm_template_test.go (new)

    • Add Test_HelmTemplate_NameAndNamespace covering accepted and rejected values, and verifying helm is not invoked when rejected
    • Add Test_HelmTemplate_NameIsNotParsedAsHelmFlag, which runs helm with a name of "--set-file=value=" and verifies the file content is not rendered. Without this change the file content appears in the helm output.

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?


Additional Notes for your reviewer:

Review Checklist:
  • Follows the developer guidelines
  • Relevant tests are added or updated
  • Relevant docs in this repo added or updated
  • Relevant carvel.dev docs added or updated in a separate PR and there's
    a link to that PR
  • Code is at least as readable and maintainable as it was before this
    change

Additional documentation e.g., Proposal, usage docs, etc.:


spec.template[].helmTemplate.name is passed to "helm template" as the
first positional argument without validation. A value starting with
'-' is parsed by helm as a flag instead of a release name, so arbitrary
helm template flags can be supplied through this field, e.g.
"--set-file=key=<path>" makes helm read a file from the controller (or
sidecarexec) container filesystem into the rendered manifests. The same
value can also be set on a PackageInstall through the
ext.packaging.carvel.dev/helm-template-name annotation.

Return an error before invoking helm when the resolved name or
namespace starts with '-'. Both values come from the App spec (or the
PackageInstall annotations), so they are validated at the point where
the helm arguments are built, which covers both paths. Valid release
names and namespaces can never start with '-', so this does not change
behavior for existing valid configurations.

Changes:

1. pkg/template/helm_template.go
   - Reject name and namespace values starting with '-'

2. pkg/template/helm_template_test.go (new)
   - Add Test_HelmTemplate_NameAndNamespace covering accepted and
     rejected values, and verifying helm is not invoked when rejected
   - Add Test_HelmTemplate_NameIsNotParsedAsHelmFlag, which runs helm
     with a name of "--set-file=value=<file>" and verifies the file
     content is not rendered. Without this change the file content
     appears in the helm output.

Signed-off-by: Himanshu Singh <himansh.singh3@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant