diff --git a/docs/remote-secrets.md b/docs/remote-secrets.md index 0b75b8ef..641f74d9 100644 --- a/docs/remote-secrets.md +++ b/docs/remote-secrets.md @@ -54,4 +54,66 @@ service: login: svc-login # fetched from vault password: pass -``` \ No newline at end of file +``` + + +## Disabling vals + +You can disable the built-in vals processing using environment variables: + +### Pass-through mode + +Set `HELMFILE_DISABLE_VALS=true` to disable internal vals processing. Any `ref+` values will pass through unchanged, allowing you to validate them with a policy tool such as [conftest](https://www.conftest.dev/) before they are resolved: + +```bash +HELMFILE_DISABLE_VALS=true helmfile template | conftest test - +``` + +### Strict mode + +Set `HELMFILE_DISABLE_VALS_STRICT=true` to disable vals and error if any `ref+` values are detected. This is useful when you want to prevent users from using vals references: + +```bash +HELMFILE_DISABLE_VALS_STRICT=true helmfile sync +# Error: vals is disabled via HELMFILE_DISABLE_VALS_STRICT environment variable +``` + +Note: If both are set, strict mode takes precedence. + +Strict mode detects any `ref+://` or `secretref+://` expression, including nested ones in maps and arrays. Plain strings that merely contain the text `ref+` (without a provider scheme) are not vals references and do not trigger the error. + +### Validating ref+ expressions with conftest + +You can use `HELMFILE_DISABLE_VALS=true` with [conftest](https://www.conftest.dev/) to validate that all `ref+` expressions conform to your security policy before processing them. + +Example rego policy (`policy/vals_refs.rego`): + +```rego +package main + +allowed_refs := { + "ref+tfstates3://my-terraform-state/networking/eu-west-2/vpc/vpc_id", + "ref+tfstates3://my-terraform-state/networking/eu-west-2/vpc/private_subnet_ids", + "ref+tfstates3://my-terraform-state/platform/eu-west-2/eks/cluster_endpoint", +} + +deny[msg] { + value := input[_] + startswith(value, "ref+tfstates3://") + not allowed_refs[value] + msg := sprintf("ref+ expression references an unapproved tfstates3 URI: %s", [value]) +} + +deny[msg] { + value := input[_] + startswith(value, "ref+") + not startswith(value, "ref+tfstates3://") + msg := sprintf("only tfstates3 ref+ expressions are permitted, got: %s", [value]) +} +``` + +Run against your rendered values: + +```bash +HELMFILE_DISABLE_VALS=true helmfile template | conftest test - +``` diff --git a/docs/templating.md b/docs/templating.md index 5c1d1b96..f0bdd477 100644 --- a/docs/templating.md +++ b/docs/templating.md @@ -62,6 +62,8 @@ Helmfile uses some OS environment variables to override default behaviour: * `HELMFILE_DISABLE_INSECURE_FEATURES` - disable insecure features, expecting `true` lower case * `HELMFILE_DISABLE_RUNNER_UNIQUE_ID` - disable unique logging ID, expecting any non-empty value +* `HELMFILE_DISABLE_VALS` - disable internal vals processing, `ref+` values pass through unchanged for use with external vals, expecting `true` lower case +* `HELMFILE_DISABLE_VALS_STRICT` - disable vals and error if any `ref+` values are detected, expecting `true` lower case * `HELMFILE_SKIP_INSECURE_TEMPLATE_FUNCTIONS` - disable insecure template functions, expecting `true` lower case * `HELMFILE_USE_HELM_STATUS_TO_CHECK_RELEASE_EXISTENCE` - expecting non-empty value to use `helm status` to check release existence, instead of `helm list` which is the default behaviour * `HELMFILE_EXPERIMENTAL` - enable experimental features, expecting `true` lower case diff --git a/pkg/envvar/const.go b/pkg/envvar/const.go index 985bd881..74f8b7e9 100644 --- a/pkg/envvar/const.go +++ b/pkg/envvar/const.go @@ -4,6 +4,10 @@ const ( DisableInsecureFeatures = "HELMFILE_DISABLE_INSECURE_FEATURES" DisableInsecureTemplateFunctions = "HELMFILE_DISABLE_INSECURE_TEMPLATE_FUNCTIONS" DisableHooks = "HELMFILE_DISABLE_HOOKS" + // DisableVals passes `ref+` values through unchanged for external vals processing + DisableVals = "HELMFILE_DISABLE_VALS" + // DisableValsStrict errors when any `ref+` value is detected + DisableValsStrict = "HELMFILE_DISABLE_VALS_STRICT" // use helm status to check if a release exists before installing it UseHelmStatusToCheckReleaseExistence = "HELMFILE_USE_HELM_STATUS_TO_CHECK_RELEASE_EXISTENCE" diff --git a/pkg/plugins/vals.go b/pkg/plugins/vals.go index f0c2767f..d73169b8 100644 --- a/pkg/plugins/vals.go +++ b/pkg/plugins/vals.go @@ -1,9 +1,11 @@ package plugins import ( + "errors" "fmt" "io" "os" + "regexp" "strconv" "strings" "sync" @@ -18,9 +20,105 @@ const ( valsCacheSize = 512 ) -var instance *vals.Runtime +var instance vals.Evaluator var mu sync.Mutex +// ErrValsDisabled is returned by the evaluator when HELMFILE_DISABLE_VALS_STRICT is set +// and a `ref+` expression is encountered. +var ErrValsDisabled = errors.New("vals is disabled via HELMFILE_DISABLE_VALS_STRICT environment variable") + +// refPlusRegexp mirrors the reference syntax understood by the vals library +// (`ref+://...` and `secretref+://...`), so that disabled-vals +// modes only report values that vals itself would have tried to resolve. +var refPlusRegexp = regexp.MustCompile(`(secret)?ref\+[^\s+:]*://`) + +// passthroughEvaluator passes values through unchanged (for external vals) +type passthroughEvaluator struct{} + +func (p *passthroughEvaluator) Eval(m map[string]any) (map[string]any, error) { + return normalizeMap(m), nil +} + +// strictEvaluator passes through values but errors if ref+ is detected +type strictEvaluator struct{} + +func (s *strictEvaluator) Eval(m map[string]any) (map[string]any, error) { + if containsRefPlus(m) { + return nil, ErrValsDisabled + } + return normalizeMap(m), nil +} + +// normalizeMap converts []string values to []any to match vals.Eval behavior. +func normalizeMap(m map[string]any) map[string]any { + out := make(map[string]any, len(m)) + for k, v := range m { + out[k] = normalizeValue(v) + } + return out +} + +// normalizeValue recursively converts []string to []any and map[any]any to +// map[string]any, matching the type normalization performed by vals.Eval. +func normalizeValue(v any) any { + switch typed := v.(type) { + case map[string]any: + return normalizeMap(typed) + case map[any]any: + strmap := make(map[string]any, len(typed)) + for k, v := range typed { + strmap[fmt.Sprintf("%v", k)] = normalizeValue(v) + } + return strmap + case []any: + a := make([]any, len(typed)) + for i, e := range typed { + a[i] = normalizeValue(e) + } + return a + case []string: + a := make([]any, len(typed)) + for i, s := range typed { + a[i] = s + } + return a + default: + return v + } +} + +func containsRefPlus(v any) bool { + switch val := v.(type) { + case string: + return refPlusRegexp.MatchString(val) + case map[string]any: + for _, v := range val { + if containsRefPlus(v) { + return true + } + } + case map[any]any: + for _, v := range val { + if containsRefPlus(v) { + return true + } + } + case []any: + for _, v := range val { + if containsRefPlus(v) { + return true + } + } + case []string: + for _, s := range val { + if refPlusRegexp.MatchString(s) { + return true + } + } + } + return false +} + func buildValsOptions() (vals.Options, error) { // Configure AWS SDK logging via HELMFILE_AWS_SDK_LOG_LEVEL environment variable // Default: "off" to prevent sensitive information (tokens, auth headers) from being exposed @@ -78,7 +176,7 @@ func buildValsOptions() (vals.Options, error) { return opts, nil } -func ValsInstance() (*vals.Runtime, error) { +func ValsInstance() (vals.Evaluator, error) { mu.Lock() defer mu.Unlock() @@ -86,6 +184,20 @@ func ValsInstance() (*vals.Runtime, error) { return instance, nil } + // HELMFILE_DISABLE_VALS_STRICT: error on ref+ usage + strict, _ := strconv.ParseBool(os.Getenv(envvar.DisableValsStrict)) + if strict { + instance = &strictEvaluator{} + return instance, nil + } + + // HELMFILE_DISABLE_VALS: pass-through for external vals + disabled, _ := strconv.ParseBool(os.Getenv(envvar.DisableVals)) + if disabled { + instance = &passthroughEvaluator{} + return instance, nil + } + opts, err := buildValsOptions() if err != nil { return nil, err diff --git a/pkg/plugins/vals_test.go b/pkg/plugins/vals_test.go index 2b98489e..8a785973 100644 --- a/pkg/plugins/vals_test.go +++ b/pkg/plugins/vals_test.go @@ -2,6 +2,7 @@ package plugins import ( "io" + "os" "testing" "github.com/helmfile/vals" @@ -11,20 +12,347 @@ import ( "github.com/helmfile/helmfile/pkg/envvar" ) -func TestValsInstance(t *testing.T) { - i, err := ValsInstance() +// resetInstance resets the singleton for testing +func resetInstance() { + mu.Lock() + defer mu.Unlock() + instance = nil +} +// setenvForTest sets the environment variable to value, or unsets it when value is empty, +// restoring the original value when the test finishes. +func setenvForTest(t *testing.T, key, value string) { + t.Helper() + if value == "" { + orig, had := os.LookupEnv(key) + os.Unsetenv(key) + t.Cleanup(func() { + if had { + os.Setenv(key, orig) + } else { + os.Unsetenv(key) + } + }) + return + } + t.Setenv(key, value) +} + +func TestValsInstance(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "") + setenvForTest(t, envvar.DisableValsStrict, "") + + i, err := ValsInstance() if err != nil { t.Errorf("unexpected error: %v", err) } i2, _ := ValsInstance() - if i != i2 { t.Error("Instances should be equal") } } +func TestDisableVals(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "true") + setenvForTest(t, envvar.DisableValsStrict, "") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Should pass through values unchanged + input := map[string]any{"key": "ref+echo://secret"} + output, err := evaluator.Eval(input) + if err != nil { + t.Fatalf("passthrough should not error: %v", err) + } + + if output["key"] != "ref+echo://secret" { + t.Errorf("expected ref+ to pass through unchanged, got %v", output["key"]) + } +} + +func TestDisableValsStrict(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "") + setenvForTest(t, envvar.DisableValsStrict, "true") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Should error on ref+ + input := map[string]any{"key": "ref+echo://secret"} + _, err = evaluator.Eval(input) + if err == nil { + t.Fatal("strict mode should error on ref+") + } + if err != ErrValsDisabled { + t.Errorf("expected ErrValsDisabled, got %v", err) + } +} + +func TestDisableValsStrictTakesPrecedence(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "true") + setenvForTest(t, envvar.DisableValsStrict, "true") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Strict mode should win over pass-through mode + input := map[string]any{"key": "ref+echo://secret"} + _, err = evaluator.Eval(input) + if err != ErrValsDisabled { + t.Errorf("expected ErrValsDisabled when both env vars are set, got %v", err) + } +} + +func TestDisableValsStrictAllowsNonRef(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "") + setenvForTest(t, envvar.DisableValsStrict, "true") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Should pass through non-ref+ values, including strings that merely + // contain "ref+" without a provider scheme (not a vals reference) + input := map[string]any{ + "key": "normal-value", + "literal": "some ref+ text without scheme", + "secretRef2": "ref+2", + } + output, err := evaluator.Eval(input) + if err != nil { + t.Fatalf("strict mode should allow non-ref+ values: %v", err) + } + if output["key"] != "normal-value" { + t.Errorf("expected value to pass through, got %v", output["key"]) + } +} + +func TestDisableValsStrictDetectsSecretRef(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "") + setenvForTest(t, envvar.DisableValsStrict, "true") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // secretref+ is also a vals reference and must be rejected + input := map[string]any{"key": "secretref+vault://secret/data/x#y"} + if _, err := evaluator.Eval(input); err != ErrValsDisabled { + t.Errorf("expected ErrValsDisabled for secretref+ expression, got %v", err) + } +} + +func TestDisableValsStrictNestedRef(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "") + setenvForTest(t, envvar.DisableValsStrict, "true") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Should detect nested ref+ in map[string]any + input := map[string]any{ + "outer": map[string]any{ + "inner": "ref+vault://secret", + }, + } + _, err = evaluator.Eval(input) + if err == nil { + t.Fatal("strict mode should detect nested ref+") + } +} + +func TestDisableValsStrictMapAnyAny(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "") + setenvForTest(t, envvar.DisableValsStrict, "true") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Should detect ref+ inside map[any]any (yaml.v2 nested maps) + input := map[string]any{ + "outer": map[any]any{ + "inner": "ref+vault://secret", + }, + } + _, err = evaluator.Eval(input) + if err == nil { + t.Fatal("strict mode should detect ref+ in map[any]any") + } +} + +func TestDisableValsStrictArrayRef(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "") + setenvForTest(t, envvar.DisableValsStrict, "true") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Should detect ref+ in []any arrays + input := map[string]any{ + "values": []any{"normal", "ref+awssecrets://db/password"}, + } + _, err = evaluator.Eval(input) + if err == nil { + t.Fatal("strict mode should detect ref+ in arrays") + } +} + +func TestDisableValsStrictStringSlice(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "") + setenvForTest(t, envvar.DisableValsStrict, "true") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Should detect ref+ in []string arrays (matches renderValsSecrets usage) + input := map[string]any{ + "values": []string{"normal", "ref+awssecrets://db/password"}, + } + _, err = evaluator.Eval(input) + if err == nil { + t.Fatal("strict mode should detect ref+ in []string arrays") + } +} + +func TestDisableValsPassThroughStringSlice(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "true") + setenvForTest(t, envvar.DisableValsStrict, "") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + input := map[string]any{ + "values": []string{"normal", "ref+awssecrets://db/password"}, + } + out, err := evaluator.Eval(input) + if err != nil { + t.Fatalf("pass-through should not error: %v", err) + } + + values, ok := out["values"].([]any) + if !ok { + t.Fatalf("expected out[\"values\"] to be []any, got %T", out["values"]) + } + if values[0] != "normal" || values[1] != "ref+awssecrets://db/password" { + t.Errorf("unexpected values: %v", values) + } +} + +func TestDisableValsPassThroughNestedTypes(t *testing.T) { + resetInstance() + defer resetInstance() + + setenvForTest(t, envvar.DisableVals, "true") + setenvForTest(t, envvar.DisableValsStrict, "") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Nested map[any]any and []string should be normalized to map[string]any + // and []any respectively, matching vals.Eval behavior + input := map[string]any{ + "outer": map[any]any{ + "list": []string{"normal", "ref+echo://secret"}, + }, + } + out, err := evaluator.Eval(input) + if err != nil { + t.Fatalf("pass-through should not error: %v", err) + } + + outer, ok := out["outer"].(map[string]any) + if !ok { + t.Fatalf("expected out[\"outer\"] to be map[string]any, got %T", out["outer"]) + } + list, ok := outer["list"].([]any) + if !ok { + t.Fatalf("expected outer[\"list\"] to be []any, got %T", outer["list"]) + } + if list[0] != "normal" || list[1] != "ref+echo://secret" { + t.Errorf("unexpected values: %v", list) + } +} + +func TestNormalValsProcessing(t *testing.T) { + resetInstance() + defer resetInstance() + + // Ensure both are unset + setenvForTest(t, envvar.DisableVals, "") + setenvForTest(t, envvar.DisableValsStrict, "") + + evaluator, err := ValsInstance() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // ref+echo should expand to the value after :// + input := map[string]any{"key": "ref+echo://myvalue"} + output, err := evaluator.Eval(input) + if err != nil { + t.Fatalf("normal vals should process ref+echo: %v", err) + } + + if output["key"] != "myvalue" { + t.Errorf("expected 'myvalue', got %v", output["key"]) + } +} + func TestBuildValsOptions(t *testing.T) { tests := []struct { name string @@ -129,14 +457,6 @@ func TestBuildValsOptions(t *testing.T) { expectedFailOnMissingKey: false, expectedLogOutputDiscarded: true, }, - { - name: "aws log level Off mixed case", - awsLogLevel: "Off", - failOnMissingKey: "", - expectedLogLevel: "off", - expectedFailOnMissingKey: false, - expectedLogOutputDiscarded: true, - }, { name: "both options set", awsLogLevel: "standard", @@ -172,6 +492,7 @@ func TestBuildValsOptions(t *testing.T) { } } +// TestAWSSDKLogLevelConfiguration tests the AWS SDK log level configuration logic func TestAWSSDKLogLevelConfiguration(t *testing.T) { tests := []struct { name string diff --git a/pkg/tmpl/expand_secret_ref.go b/pkg/tmpl/expand_secret_ref.go index bf3255b2..224ee9f8 100644 --- a/pkg/tmpl/expand_secret_ref.go +++ b/pkg/tmpl/expand_secret_ref.go @@ -4,8 +4,6 @@ import ( "fmt" "sync" - "github.com/helmfile/vals" - "github.com/helmfile/helmfile/pkg/plugins" ) @@ -42,13 +40,8 @@ func fetchSecretValues(values map[string]any) (map[string]any, error) { var err error // below lines are for tests once.Do(func() { - var valRuntime *vals.Runtime if secretsClient == nil { - valRuntime, err = plugins.ValsInstance() - if err != nil { - return - } - secretsClient = valRuntime + secretsClient, err = plugins.ValsInstance() } }) if secretsClient == nil {