diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go index 11b22434ea..afe65ebcc5 100644 --- a/internal/backend/local/test.go +++ b/internal/backend/local/test.go @@ -19,6 +19,7 @@ import ( "github.com/hashicorp/terraform/internal/backend" "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/logging" "github.com/hashicorp/terraform/internal/moduletest" configtest "github.com/hashicorp/terraform/internal/moduletest/config" @@ -395,7 +396,12 @@ func (runner *TestFileRunner) run(run *moduletest.Run, file *moduletest.File, st return state, false } - planCtx, plan, planDiags := runner.plan(config, state, run, file, runner.FilterVariablesToConfig(config, variables), references, start) + // FilterVariablesToConfig only returns warnings, so we don't check the + // returned diags for errors. + setVariables, setVariableDiags := runner.FilterVariablesToConfig(config, variables) + run.Diagnostics = run.Diagnostics.Append(setVariableDiags) + + planCtx, plan, planDiags := runner.plan(config, state, run, file, setVariables, references, start) if run.Config.Command == configs.PlanTestCommand { // Then we want to assess our conditions and diagnostics differently. planDiags = run.ValidateExpectedFailures(planDiags) @@ -586,9 +592,15 @@ func (runner *TestFileRunner) destroy(config *configs.Config, state *states.Stat return state, diags } + // During the destroy operation, we don't add warnings from this operation. + // Anything that would have been reported here was already reported during + // the original plan, and a successful destroy operation is the only thing + // we care about. + setVariables, _ := runner.FilterVariablesToConfig(config, variables) + planOpts := &terraform.PlanOpts{ Mode: plans.DestroyMode, - SetVariables: runner.FilterVariablesToConfig(config, variables), + SetVariables: setVariables, } tfCtx, ctxDiags := terraform.NewContext(runner.Suite.Opts) @@ -1140,17 +1152,46 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete // This function is essentially the opposite of AddVariablesToConfig which // makes the config match the variables rather than the variables match the // config. -func (runner *TestFileRunner) FilterVariablesToConfig(config *configs.Config, values terraform.InputValues) terraform.InputValues { +// +// This function can only return warnings, and the callers can rely on this so +// please check the callers of this function if you add any error diagnostics. +func (runner *TestFileRunner) FilterVariablesToConfig(config *configs.Config, values terraform.InputValues) (terraform.InputValues, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + filtered := make(terraform.InputValues) for name, value := range values { - if _, exists := config.Module.Variables[name]; !exists { + variableConfig, exists := config.Module.Variables[name] + if !exists { // Only include values that are actually required by the config. continue } + if marks.Has(value.Value, marks.Sensitive) { + unmarkedValue, _ := value.Value.Unmark() + if !variableConfig.Sensitive { + // Then we are passing a sensitive value into a non-sensitive + // variable. Let's add a warning and tell the user they should + // mark the config as sensitive as well. If the config variable + // is sensitive, then we don't need to worry. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Sensitive metadata on variable lost", + Detail: fmt.Sprintf("The input variable is marked as sensitive, while the receiving configuration is not. The underlying sensitive information may be exposed when var.%s is referenced. Mark the variable block in the configuration as sensitive to resolve this warning.", variableConfig.Name), + Subject: value.SourceRange.ToHCL().Ptr(), + }) + } + + // Set the unmarked value into the input value. + value = &terraform.InputValue{ + Value: unmarkedValue, + SourceType: value.SourceType, + SourceRange: value.SourceRange, + } + } + filtered[name] = value } - return filtered + return filtered, diags } // AddVariablesToConfig extends the provided config to ensure it has definitions diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 6435c928fe..ed713e85b9 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -1573,6 +1573,80 @@ operation, and the specified output value is only known after apply. } +func TestTest_SensitiveInputValues(t *testing.T) { + td := t.TempDir() + testCopyDir(t, testFixturePath(path.Join("test", "sensitive_input_values")), td) + defer testChdir(t, td)() + + provider := testing_command.NewProvider(nil) + + providerSource, close := newMockProviderSource(t, map[string][]string{ + "test": {"1.0.0"}, + }) + defer close() + + streams, done := terminal.StreamsForTesting(t) + view := views.NewView(streams) + ui := new(cli.MockUi) + + meta := Meta{ + testingOverrides: metaOverridesForProvider(provider.Provider), + Ui: ui, + View: view, + Streams: streams, + ProviderSource: providerSource, + } + + init := &InitCommand{ + Meta: meta, + } + + if code := init.Run(nil); code != 0 { + t.Fatalf("expected status code 0 but got %d: %s", code, ui.ErrorWriter) + } + + c := &TestCommand{ + Meta: meta, + } + + code := c.Run([]string{"-no-color"}) + output := done(t) + + if code != 0 { + t.Errorf("expected status code 0 but got %d", code) + } + + expected := `main.tftest.hcl... in progress + run "setup"... pass + run "test"... pass + +Warning: Sensitive metadata on variable lost + + on main.tftest.hcl line 13, in run "test": + 13: password = run.setup.password + +The input variable is marked as sensitive, while the receiving configuration +is not. The underlying sensitive information may be exposed when var.password +is referenced. Mark the variable block in the configuration as sensitive to +resolve this warning. + +main.tftest.hcl... tearing down +main.tftest.hcl... pass + +Success! 2 passed, 0 failed. +` + + actual := output.All() + + if diff := cmp.Diff(actual, expected); len(diff) > 0 { + t.Errorf("output didn't match expected:\nexpected:\n%s\nactual:\n%s\ndiff:\n%s", expected, actual, diff) + } + + if provider.ResourceCount() > 0 { + t.Errorf("should have deleted all resources on completion but left %v", provider.ResourceString()) + } +} + // This test takes around 10 seconds to complete, as we're testing the progress // updates that are printed every 2 seconds. Sorry! func TestTest_LongRunningTest(t *testing.T) { diff --git a/internal/command/testdata/test/sensitive_input_values/main.tf b/internal/command/testdata/test/sensitive_input_values/main.tf new file mode 100644 index 0000000000..6d61c0c241 --- /dev/null +++ b/internal/command/testdata/test/sensitive_input_values/main.tf @@ -0,0 +1,8 @@ +variable "password" { + type = string +} + +output "password" { + value = var.password + sensitive = true +} diff --git a/internal/command/testdata/test/sensitive_input_values/main.tftest.hcl b/internal/command/testdata/test/sensitive_input_values/main.tftest.hcl new file mode 100644 index 0000000000..ad2c16bde8 --- /dev/null +++ b/internal/command/testdata/test/sensitive_input_values/main.tftest.hcl @@ -0,0 +1,15 @@ +run "setup" { + variables { + password = "password" + } + + module { + source = "./setup" + } +} + +run "test" { + variables { + password = run.setup.password + } +} diff --git a/internal/command/testdata/test/sensitive_input_values/setup/main.tf b/internal/command/testdata/test/sensitive_input_values/setup/main.tf new file mode 100644 index 0000000000..0b0576edb8 --- /dev/null +++ b/internal/command/testdata/test/sensitive_input_values/setup/main.tf @@ -0,0 +1,9 @@ +variable "password" { + sensitive = true + type = string +} + +output "password" { + value = var.password + sensitive = true +}