diff --git a/CHANGELOG.md b/CHANGELOG.md index 04bda774b2..5b9e5f098b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## 1.8.7 (unreleased) +BUG FIXES: +* Error messages related to validation of sensitive input variables will no longer disclose the sensitive value in the UI. ([#2219](https://github.com/opentofu/opentofu/pull/2219)) ## 1.8.6 diff --git a/internal/tofu/eval_variable.go b/internal/tofu/eval_variable.go index 1033033db2..a3f8353fe6 100644 --- a/internal/tofu/eval_variable.go +++ b/internal/tofu/eval_variable.go @@ -235,6 +235,13 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config }) return diags } + if config.Sensitive { + // Normally this marking is handled automatically during construction + // of the HCL eval context the evaluation scope, but this codepath + // augments that scope with the not-yet-finalized input variable + // value, so we need to apply this mark separately. + val = val.Mark(marks.Sensitive) + } for ix, validation := range config.Validations { condFuncs, condDiags := lang.ProviderFunctionsInExpr(addrs.ParseRef, validation.Condition) diags = diags.Append(condDiags) diff --git a/internal/tofu/eval_variable_test.go b/internal/tofu/eval_variable_test.go index 5f7fd87f29..10b07be20d 100644 --- a/internal/tofu/eval_variable_test.go +++ b/internal/tofu/eval_variable_test.go @@ -1332,11 +1332,11 @@ variable "bar" { if got, want := addr.String(), varAddr.String(); got != want { t.Errorf("incorrect argument to GetVariableValue: got %s, want %s", got, want) } - if varCfg.Sensitive { - return test.given.Mark(marks.Sensitive) - } else { - return test.given - } + // NOTE: This intentionally doesn't mark the result as sensitive, + // because BuiltinEvalContext.GetVariableValueFunc doesn't either. + // It's the responsibility of downstream code to detect and handle + // configured sensitivity. + return test.given } ctx.ChecksState = checks.NewState(cfg) ctx.ChecksState.ReportCheckableObjects(varAddr.ConfigCheckable(), addrs.MakeSet[addrs.Checkable](varAddr)) @@ -1371,3 +1371,76 @@ variable "bar" { }) } } + +func TestEvalVariableValidations_sensitiveValueDiagnostics(t *testing.T) { + // This test verifies that values for sensitive variables get captured + // into diagnostic messages with the sensitive mark intact, so that + // the values won't be disclosed in the UI. + // Earlier versions handled this incorrectly: + // https://github.com/opentofu/opentofu/issues/2219 + + cfgSrc := ` + variable "foo" { + type = string + sensitive = true + + validation { + condition = length(var.foo) == 8 # intentionally fails + error_message = "Foo must have 8 characters." + } + } + ` + cfg := testModuleInline(t, map[string]string{ + "main.tf": cfgSrc, + }) + varAddr := addrs.InputVariable{Name: "foo"}.Absolute(addrs.RootModuleInstance) + + ctx := &MockEvalContext{} + ctx.EvaluationScopeScope = &lang.Scope{} + ctx.GetVariableValueFunc = func(addr addrs.AbsInputVariableInstance) cty.Value { + if got, want := addr.String(), varAddr.String(); got != want { + t.Errorf("incorrect argument to GetVariableValue: got %s, want %s", got, want) + } + // NOTE: This intentionally doesn't mark the result as sensitive, + // because BuiltinEvalContext.GetVariableValueFunc doesn't either. + // It's the responsibility of downstream code to detect and handle + // configured sensitivity. + return cty.StringVal("boop") + } + ctx.ChecksState = checks.NewState(cfg) + ctx.ChecksState.ReportCheckableObjects(varAddr.ConfigCheckable(), addrs.MakeSet[addrs.Checkable](varAddr)) + + gotDiags := evalVariableValidations( + varAddr, cfg.Module.Variables["foo"], nil, ctx, + ) + if !gotDiags.HasErrors() { + t.Fatalf("unexpected success; want validation error") + } + + // The generated diagnostic(s) should all capture the evaluation context + // that was used to evaluate the condition, where the variable's value + // should be marked as sensitive so it won't get displayed in clear + // in the UI output. + // (The HasErrors check above guarantees that there's at least one + // diagnostic here for us to iterate over.) + for _, diag := range gotDiags { + if diag.Severity() != tfdiags.Error { + continue + } + fromExpr := diag.FromExpr() + if fromExpr == nil { + t.Fatalf("diagnostic does not have source expression information at all") + } + allVarVals := fromExpr.EvalContext.Variables["var"] + if allVarVals == cty.NilVal || !allVarVals.Type().IsObjectType() { + t.Fatalf("diagnostic did not capture an object value for the top-level symbol 'var'") + } + gotVal := allVarVals.GetAttr("foo") + if gotVal == cty.NilVal { + t.Fatalf("diagnostic did not capture a value for var.foo") + } + if !gotVal.HasMark(marks.Sensitive) { + t.Errorf("var.foo value is not marked as sensitive in diagnostic") + } + } +}