Merge pull request #30552 from gbataille/29156_do_not_log_sensitive_v…

…alues

Fixes #29156: Failing sensitive variables values are not logged

internal/terraform/eval_variable.go

@@ -93,27 +93,40 @@ func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *In
 	val, err := convert.Convert(given, convertTy)
 	if err != nil {
 		log.Printf("[ERROR] prepareFinalInputVariableValue: %s has unsuitable type\n  got:  %s\n  want: %s", addr, given.Type(), convertTy)
+		var detail string
+		var subject *hcl.Range
 		if nonFileSource != "" {
-			diags = diags.Append(&hcl.Diagnostic{
-				Severity: hcl.DiagError,
-				Summary:  "Invalid value for input variable",
-				Detail: fmt.Sprintf(
-					"Unsuitable value for %s %s: %s.",
-					addr, nonFileSource, err,
-				),
-				Subject: cfg.DeclRange.Ptr(),
-			})
+			detail = fmt.Sprintf(
+				"Unsuitable value for %s %s: %s.",
+				addr, nonFileSource, err,
+			)
+			subject = cfg.DeclRange.Ptr()
 		} else {
-			diags = diags.Append(&hcl.Diagnostic{
-				Severity: hcl.DiagError,
-				Summary:  "Invalid value for input variable",
-				Detail: fmt.Sprintf(
-					"The given value is not suitable for %s declared at %s: %s.",
-					addr, cfg.DeclRange.String(), err,
-				),
-				Subject: sourceRange.ToHCL().Ptr(),
-			})
+			detail = fmt.Sprintf(
+				"The given value is not suitable for %s declared at %s: %s.",
+				addr, cfg.DeclRange.String(), err,
+			)
+			subject = sourceRange.ToHCL().Ptr()
+
+			// In some workflows, the operator running terraform does not have access to the variables
+			// themselves. They are for example stored in encrypted files that will be used by the CI toolset
+			// and not by the operator directly. In such a case, the failing secret value should not be
+			// displayed to the operator
+			if cfg.Sensitive {
+				detail = fmt.Sprintf(
+					"The given value is not suitable for %s, which is sensitive: %s. Invalid value defined at %s.",
+					addr, err, sourceRange.ToHCL(),
+				)
+				subject = cfg.DeclRange.Ptr()
+			}
 		}
+
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Invalid value for input variable",
+			Detail:   detail,
+			Subject:  subject,
+		})
 		// We'll return a placeholder unknown value to avoid producing
 		// redundant downstream errors.
 		return cty.UnknownVal(cfg.Type), diags

internal/terraform/eval_variable_test.go

@@ -63,6 +63,11 @@ func TestPrepareFinalInputVariableValue(t *testing.T) {
 			type     = string
 			default  = true
 		}
+		variable "constrained_string_sensitive_required" {
+			sensitive = true
+			nullable  = false
+			type      = string
+		}
 	`
 	cfg := testModuleInline(t, map[string]string{
 		"main.tf": cfgSrc,
@@ -393,6 +398,14 @@ func TestPrepareFinalInputVariableValue(t *testing.T) {
 			cty.UnknownVal(cty.String),
 			``,
 		},
+
+		// sensitive
+		{
+			"constrained_string_sensitive_required",
+			cty.UnknownVal(cty.String),
+			cty.UnknownVal(cty.String),
+			``,
+		},
 	}
 
 	for _, test := range tests {
@@ -563,6 +576,63 @@ func TestPrepareFinalInputVariableValue(t *testing.T) {
 			})
 		}
 	})
+
+	t.Run("SensitiveVariable error message variants, with source variants", func(t *testing.T) {
+		tests := []struct {
+			SourceType  ValueSourceType
+			SourceRange tfdiags.SourceRange
+			WantTypeErr string
+			HideSubject bool
+		}{
+			{
+				ValueFromUnknown,
+				tfdiags.SourceRange{},
+				"Invalid value for input variable: Unsuitable value for var.constrained_string_sensitive_required set from outside of the configuration: string required.",
+				false,
+			},
+			{
+				ValueFromConfig,
+				tfdiags.SourceRange{
+					Filename: "example.tfvars",
+					Start:    tfdiags.SourcePos(hcl.InitialPos),
+					End:      tfdiags.SourcePos(hcl.InitialPos),
+				},
+				`Invalid value for input variable: The given value is not suitable for var.constrained_string_sensitive_required, which is sensitive: string required. Invalid value defined at example.tfvars:1,1-1.`,
+				true,
+			},
+		}
+
+		for _, test := range tests {
+			t.Run(fmt.Sprintf("%s %s", test.SourceType, test.SourceRange.StartString()), func(t *testing.T) {
+				varAddr := addrs.InputVariable{Name: "constrained_string_sensitive_required"}.Absolute(addrs.RootModuleInstance)
+				varCfg := variableConfigs[varAddr.Variable.Name]
+				t.Run("type error", func(t *testing.T) {
+					rawVal := &InputValue{
+						Value:       cty.EmptyObjectVal,
+						SourceType:  test.SourceType,
+						SourceRange: test.SourceRange,
+					}
+
+					_, diags := prepareFinalInputVariableValue(
+						varAddr, rawVal, varCfg,
+					)
+					if !diags.HasErrors() {
+						t.Fatalf("unexpected success; want error")
+					}
+
+					if got, want := diags.Err().Error(), test.WantTypeErr; got != want {
+						t.Errorf("wrong error\ngot:  %s\nwant: %s", got, want)
+					}
+
+					if test.HideSubject {
+						if got, want := diags[0].Source().Subject.StartString(), test.SourceRange.StartString(); got == want {
+							t.Errorf("Subject start should have been hidden, but was %s", got)
+						}
+					}
+				})
+			})
+		}
+	})
 }
 
 // These tests cover the JSON syntax configuration edge case handling,