tofu: Variable validation diagnostics must mark sensitive values

The existing test for input variable validation with sensitive values was
building on a false premise that sensitive-flagged variables always emerge
from EvalContext.GetVariableValue as marked. In practice that's true only
for input variables passed from a parent model where the value was already
marked on entry, because the marking caused directly by the
"sensitive = true" argument is handled later once the value has been
finalized and is being reported from the main expression scope.

Since the variable validation codepath writes a not-yet-finalized value
for the variable into its own HCL expression scope directly, it needs to
apply the sensitive mark itself when the variable was configured as
sensitive, which therefore means that the value provided to the "condition"
expression is properly marked and any diagnostics generated from it will
capture that marked value.

Along with fixing the existing test to have a mock GetVariableValue that
more realistically models BuiltinEvalContext's behavior, this introduces
a new test specifically about the marking of the values in the diagnostic
errors to help avoid potentially regressing that specific detail, since
diagnostics there can potentially originate from various different parts
of the system.

Signed-off-by: Martin Atkins <mart@degeneration.co.uk>

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

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)

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")
+		}
+	}
+}