Add sensitive attribute to variables

command/format/diagnostic.go

@@ -302,6 +302,10 @@ func compactValueStr(val cty.Value) string {
 	// helpful but concise messages in diagnostics. It is not comprehensive
 	// nor intended to be used for other purposes.
 
+	if val.ContainsMarked() {
+		return "(sensitive value)"
+	}
+
 	ty := val.Type()
 	switch {
 	case val.IsNull():

command/format/diff.go

@@ -125,10 +125,18 @@ func ResourceChange(
 	changeV.Change.Before = objchange.NormalizeObjectFromLegacySDK(changeV.Change.Before, schema)
 	changeV.Change.After = objchange.NormalizeObjectFromLegacySDK(changeV.Change.After, schema)
 
+	// Now that the change is decoded, add back the marks at the defined paths
+	if len(change.BeforeValMarks) > 0 {
+		changeV.Change.Before = changeV.Change.Before.MarkWithPaths(change.BeforeValMarks)
+	}
+	if len(change.AfterValMarks) > 0 {
+		changeV.Change.After = changeV.Change.After.MarkWithPaths(change.AfterValMarks)
+	}
+
 	result := p.writeBlockBodyDiff(schema, changeV.Before, changeV.After, 6, path)
 	if result.bodyWritten {
-		p.buf.WriteString("\n")
-		p.buf.WriteString(strings.Repeat(" ", 4))
+		buf.WriteString("\n")
+		buf.WriteString(strings.Repeat(" ", 4))
 	}
 	buf.WriteString("}\n")
 
@@ -293,10 +301,18 @@ func (p *blockBodyDiffPrinter) writeBlockBodyDiff(schema *configschema.Block, ol
 	return result
 }
 
-func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.Attribute, old, new cty.Value, nameLen, indent int, path cty.Path) bool {
-	path = append(path, cty.GetAttrStep{Name: name})
-	showJustNew := false
+// getPlanActionAndShow returns the action value
+// and a boolean for showJustNew. In this function we
+// modify the old and new values to remove any possible marks
+func getPlanActionAndShow(old cty.Value, new cty.Value) (plans.Action, bool) {
 	var action plans.Action
+	showJustNew := false
+	if old.ContainsMarked() {
+		old, _ = old.UnmarkDeep()
+	}
+	if new.ContainsMarked() {
+		new, _ = new.UnmarkDeep()
+	}
 	switch {
 	case old.IsNull():
 		action = plans.Create
@@ -309,6 +325,12 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
 	default:
 		action = plans.Update
 	}
+	return action, showJustNew
+}
+
+func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.Attribute, old, new cty.Value, nameLen, indent int, path cty.Path) bool {
+	path = append(path, cty.GetAttrStep{Name: name})
+	action, showJustNew := getPlanActionAndShow(old, new)
 
 	if action == plans.NoOp && p.concise && !identifyingAttribute(name, attrS) {
 		return true
@@ -586,6 +608,12 @@ func (p *blockBodyDiffPrinter) writeNestedBlockDiff(name string, label *string,
 }
 
 func (p *blockBodyDiffPrinter) writeValue(val cty.Value, action plans.Action, indent int) {
+	// Could check specifically for the sensitivity marker
+	if val.IsMarked() {
+		p.buf.WriteString("(sensitive)")
+		return
+	}
+
 	if !val.IsKnown() {
 		p.buf.WriteString("(known after apply)")
 		return
@@ -739,6 +767,12 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
 	ty := old.Type()
 	typesEqual := ctyTypesEqual(ty, new.Type())
 
+	// If either the old or new value is marked, don't display the value
+	if old.ContainsMarked() || new.ContainsMarked() {
+		p.buf.WriteString("(sensitive)")
+		return
+	}
+
 	// We have some specialized diff implementations for certain complex
 	// values where it's useful to see a visualization of the diff of
 	// the nested elements rather than just showing the entire old and
@@ -1284,7 +1318,8 @@ func ctyGetAttrMaybeNull(val cty.Value, name string) cty.Value {
 	// This allows us to avoid spurious diffs
 	// until we introduce null to the SDK.
 	attrValue := val.GetAttr(name)
-	if ctyEmptyString(attrValue) {
+	// If the value is marked, the ctyEmptyString function will fail
+	if !val.ContainsMarked() && ctyEmptyString(attrValue) {
 		return cty.NullVal(attrType)
 	}
 

command/format/diff_test.go

@@ -3622,10 +3622,75 @@ func TestResourceChange_nestedMap(t *testing.T) {
 	runTestCases(t, testCases)
 }
 
+func TestResourceChange_sensitiveVariable(t *testing.T) {
+	testCases := map[string]testCase{
+		"in-place update - creation": {
+			Action: plans.Update,
+			Mode:   addrs.ManagedResourceMode,
+			Before: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-BEFORE"),
+				"root_block_device": cty.MapValEmpty(cty.Object(map[string]cty.Type{
+					"volume_type": cty.String,
+				})),
+			}),
+			After: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-AFTER"),
+				"root_block_device": cty.MapVal(map[string]cty.Value{
+					"a": cty.ObjectVal(map[string]cty.Value{
+						"volume_type": cty.StringVal("gp2"),
+					}),
+				}),
+			}),
+			AfterValMarks: []cty.PathValueMarks{
+				{
+					Path:  cty.Path{cty.GetAttrStep{Name: "ami"}},
+					Marks: cty.NewValueMarks("sensitive"),
+				}},
+			RequiredReplace: cty.NewPathSet(),
+			Tainted:         false,
+			Schema: &configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"id":  {Type: cty.String, Optional: true, Computed: true},
+					"ami": {Type: cty.String, Optional: true},
+				},
+				BlockTypes: map[string]*configschema.NestedBlock{
+					"root_block_device": {
+						Block: configschema.Block{
+							Attributes: map[string]*configschema.Attribute{
+								"volume_type": {
+									Type:     cty.String,
+									Optional: true,
+									Computed: true,
+								},
+							},
+						},
+						Nesting: configschema.NestingMap,
+					},
+				},
+			},
+			ExpectedOutput: `  # test_instance.example will be updated in-place
+  ~ resource "test_instance" "example" {
+      ~ ami = (sensitive)
+        id  = "i-02ae66f368e8518a9"
+
+      + root_block_device "a" {
+          + volume_type = "gp2"
+        }
+    }
+`,
+		},
+	}
+	runTestCases(t, testCases)
+}
+
 type testCase struct {
 	Action          plans.Action
 	Mode            addrs.ResourceMode
 	Before          cty.Value
+	BeforeValMarks  []cty.PathValueMarks
+	AfterValMarks   []cty.PathValueMarks
 	After           cty.Value
 	Schema          *configschema.Block
 	RequiredReplace cty.PathSet
@@ -3679,9 +3744,11 @@ func runTestCases(t *testing.T, testCases map[string]testCase) {
 					Module:   addrs.RootModule,
 				},
 				ChangeSrc: plans.ChangeSrc{
-					Action: tc.Action,
-					Before: before,
-					After:  after,
+					Action:         tc.Action,
+					Before:         before,
+					After:          after,
+					BeforeValMarks: tc.BeforeValMarks,
+					AfterValMarks:  tc.AfterValMarks,
 				},
 				RequiredReplace: tc.RequiredReplace,
 			}

configs/experiments.go

@@ -138,6 +138,17 @@ func checkModuleExperiments(m *Module) hcl.Diagnostics {
 			}
 		}
 	*/
-
+	if !m.ActiveExperiments.Has(experiments.SensitiveVariables) {
+		for _, v := range m.Variables {
+			if v.Sensitive {
+				diags = diags.Append(&hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "Variable sensitivity is experimental",
+					Detail:   "This feature is currently an opt-in experiment, subject to change in future releases based on feedback.\n\nActivate the feature for this module by adding sensitive_variables to the list of active experiments.",
+					Subject:  v.DeclRange.Ptr(),
+				})
+			}
+		}
+	}
 	return diags
 }

configs/named_values.go

@@ -25,6 +25,7 @@ type Variable struct {
 	Type        cty.Type
 	ParsingMode VariableParsingMode
 	Validations []*VariableValidation
+	Sensitive   bool
 
 	DescriptionSet bool
 
@@ -94,6 +95,11 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
 		v.ParsingMode = parseMode
 	}
 
+	if attr, exists := content.Attributes["sensitive"]; exists {
+		valDiags := gohcl.DecodeExpression(attr.Expr, nil, &v.Sensitive)
+		diags = append(diags, valDiags...)
+	}
+
 	if attr, exists := content.Attributes["default"]; exists {
 		val, valDiags := attr.Expr.Value(nil)
 		diags = append(diags, valDiags...)
@@ -534,6 +540,9 @@ var variableBlockSchema = &hcl.BodySchema{
 		{
 			Name: "type",
 		},
+		{
+			Name: "sensitive",
+		},
 	},
 	Blocks: []hcl.BlockHeaderSchema{
 		{

configs/testdata/warning-files/variables-sensitive.tf

@@ -0,0 +1,7 @@
+terraform {
+  experiments = [sensitive_variables] # WARNING: Experimental feature "sensitive_variables" is active
+}
+
+variable "sensitive-value" {
+  sensitive = true
+}

experiments/experiment.go

@@ -14,12 +14,14 @@ type Experiment string
 // identifier so that it can be specified in configuration.
 const (
 	VariableValidation = Experiment("variable_validation")
+	SensitiveVariables = Experiment("sensitive_variables")
 )
 
 func init() {
 	// Each experiment constant defined above must be registered here as either
 	// a current or a concluded experiment.
 	registerConcludedExperiment(VariableValidation, "Custom variable validation can now be used by default, without enabling an experiment.")
+	registerCurrentExperiment(SensitiveVariables)
 }
 
 // GetCurrent takes an experiment name and returns the experiment value

go.mod

@@ -123,7 +123,7 @@ require (
 	github.com/xanzy/ssh-agent v0.2.1
 	github.com/xiang90/probing v0.0.0-20160813154853-07dd2e8dfe18 // indirect
 	github.com/xlab/treeprint v0.0.0-20161029104018-1d6e34225557
-	github.com/zclconf/go-cty v1.6.1
+	github.com/zclconf/go-cty v1.6.2-0.20200908203537-4ad5e68430d3
 	github.com/zclconf/go-cty-yaml v1.0.2
 	go.uber.org/atomic v1.3.2 // indirect
 	go.uber.org/multierr v1.1.0 // indirect

go.sum

@@ -503,6 +503,8 @@ github.com/zclconf/go-cty v1.1.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLE
 github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8=
 github.com/zclconf/go-cty v1.6.1 h1:wHtZ+LSSQVwUSb+XIJ5E9hgAQxyWATZsAWT+ESJ9dQ0=
 github.com/zclconf/go-cty v1.6.1/go.mod h1:VDR4+I79ubFBGm1uJac1226K5yANQFHeauxPBoP54+o=
+github.com/zclconf/go-cty v1.6.2-0.20200908203537-4ad5e68430d3 h1:iGouBJrrvGf/H4L6a2n7YBCO0FDhq81FEHI4ILDphkw=
+github.com/zclconf/go-cty v1.6.2-0.20200908203537-4ad5e68430d3/go.mod h1:VDR4+I79ubFBGm1uJac1226K5yANQFHeauxPBoP54+o=
 github.com/zclconf/go-cty-yaml v1.0.2 h1:dNyg4QLTrv2IfJpm7Wtxi55ed5gLGOlPrZ6kMd51hY0=
 github.com/zclconf/go-cty-yaml v1.0.2/go.mod h1:IP3Ylp0wQpYm50IHK8OZWKMu6sPJIUgKa8XhiVHura0=
 go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=

plans/changes.go

@@ -337,18 +337,33 @@ type Change struct {
 // to call the corresponding Encode method of that struct rather than working
 // directly with its embedded Change.
 func (c *Change) Encode(ty cty.Type) (*ChangeSrc, error) {
-	beforeDV, err := NewDynamicValue(c.Before, ty)
+	// Storing unmarked values so that we can encode unmarked values
+	// and save the PathValueMarks for re-marking the values later
+	var beforeVM, afterVM []cty.PathValueMarks
+	unmarkedBefore := c.Before
+	unmarkedAfter := c.After
+
+	if c.Before.ContainsMarked() {
+		unmarkedBefore, beforeVM = c.Before.UnmarkDeepWithPaths()
+	}
+	beforeDV, err := NewDynamicValue(unmarkedBefore, ty)
 	if err != nil {
 		return nil, err
 	}
-	afterDV, err := NewDynamicValue(c.After, ty)
+
+	if c.After.ContainsMarked() {
+		unmarkedAfter, afterVM = c.After.UnmarkDeepWithPaths()
+	}
+	afterDV, err := NewDynamicValue(unmarkedAfter, ty)
 	if err != nil {
 		return nil, err
 	}
 
 	return &ChangeSrc{
-		Action: c.Action,
-		Before: beforeDV,
-		After:  afterDV,
+		Action:         c.Action,
+		Before:         beforeDV,
+		After:          afterDV,
+		BeforeValMarks: beforeVM,
+		AfterValMarks:  afterVM,
 	}, nil
 }

plans/changes_src.go

@@ -156,6 +156,13 @@ type ChangeSrc struct {
 	// but have not yet been decoded from the serialized value used for
 	// storage.
 	Before, After DynamicValue
+
+	// BeforeValMarks and AfterValMarks are stored path+mark combinations
+	// that might be discovered when encoding a change. Marks are removed
+	// to enable encoding (marked values cannot be marshalled), and so storing
+	// the path+mark combinations allow us to re-mark the value later
+	// when, for example, displaying the diff to the UI.
+	BeforeValMarks, AfterValMarks []cty.PathValueMarks
 }
 
 // Decode unmarshals the raw representations of the before and after values

plans/objchange/compatible.go

@@ -51,7 +51,17 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu
 		actualV := actual.GetAttr(name)
 
 		path := append(path, cty.GetAttrStep{Name: name})
-		moreErrs := assertValueCompatible(plannedV, actualV, path)
+		// If our value is marked, unmark it here before
+		// checking value assertions
+		unmarkedActualV := actualV
+		if actualV.ContainsMarked() {
+			unmarkedActualV, _ = actualV.UnmarkDeep()
+		}
+		unmarkedPlannedV := plannedV
+		if plannedV.ContainsMarked() {
+			unmarkedPlannedV, _ = actualV.UnmarkDeep()
+		}
+		moreErrs := assertValueCompatible(unmarkedPlannedV, unmarkedActualV, path)
 		if attrS.Sensitive {
 			if len(moreErrs) > 0 {
 				// Use a vague placeholder message instead, to avoid disclosing

states/instance_object.go

@@ -98,7 +98,12 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res
 	// and raise an error about that.
 	val := cty.UnknownAsNull(o.Value)
 
-	src, err := ctyjson.Marshal(val, ty)
+	// If it contains marks, dump those now
+	unmarked := val
+	if val.ContainsMarked() {
+		unmarked, _ = val.UnmarkDeep()
+	}
+	src, err := ctyjson.Marshal(unmarked, ty)
 	if err != nil {
 		return nil, err
 	}

terraform/context.go

@@ -503,7 +503,6 @@ Note that the -target option is not suitable for routine use, and is provided on
 func (c *Context) Plan() (*plans.Plan, tfdiags.Diagnostics) {
 	defer c.acquireRun("plan")()
 	c.changes = plans.NewChanges()
-
 	var diags tfdiags.Diagnostics
 
 	if len(c.targets) > 0 {
@@ -575,6 +574,7 @@ The -target option is not for routine use, and is provided only for exceptional
 	diags = diags.Append(walker.NonFatalDiagnostics)
 	diags = diags.Append(walkDiags)
 	if walkDiags.HasErrors() {
+		fmt.Println("walkerr")
 		return nil, diags
 	}
 	p.Changes = c.changes