Store sensitive attribute paths in state (#26338)

* Add creation test and simplify in-place test

* Add deletion test

* Start adding marking from state

Start storing paths that should be marked
when pulled out of state. Implements deep
copy for attr paths. This commit also includes some
comment noise from investigations, and fixing the diff test

* Fix apply stripping marks

* Expand diff tests

* Basic apply test

* Update comments on equality checks to clarify current understanding

* Add JSON serialization for sensitive paths

We need to serialize a slice of cty.Path values to be used to re-mark
the sensitive values of a resource instance when loading the state file.
Paths consist of a list of steps, each of which may be either getting an
attribute value by name, or indexing into a collection by string or
number.

To serialize these without building a complex parser for a compact
string form, we render a nested array of small objects, like so:

[
  [
    { type: "get_attr", value: "foo" },
    { type: "index", value: { "type": "number", "value": 2 } }
  ]
]

The above example is equivalent to a path `foo[2]`.

* Format diffs with map types

Comparisons need unmarked values to operate on,
so create unmarked values for those operations. Additionally,
change diff to cover map types

* Remove debugging printing

* Fix bug with marking non-sensitive values

When pulling a sensitive value from state,
we were previously using those marks to remark
the planned new value, but that new value
might *not* be sensitive, so let's not do that

* Fix apply test

Apply was not passing the second state
through to the third pass at apply

* Consistency in checking for length of paths vs inspecting into value

* In apply, don't mark with before paths

* AttrPaths test coverage for DeepCopy

* Revert format changes

Reverts format changes in format/diff for this
branch so those changes can be discussed on a separate PR

* Refactor name of AttrPaths to AttrSensitivePaths

* Rename AttributePaths/attributePaths for naming consistency

Co-authored-by: Alisdair McDiarmid <alisdair@users.noreply.github.com>

command/format/diff_test.go

@@ -3624,24 +3624,13 @@ func TestResourceChange_nestedMap(t *testing.T) {
 
 func TestResourceChange_sensitiveVariable(t *testing.T) {
 	testCases := map[string]testCase{
-		"in-place update - creation": {
+		"creation": {
-			Action: plans.Update,
+			Action: plans.Create,
 			Mode:   addrs.ManagedResourceMode,
-			Before: cty.ObjectVal(map[string]cty.Value{
+			Before: cty.NullVal(cty.EmptyObject),
-				"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"),
+				"ami": cty.StringVal("ami-123"),
-				"root_block_device": cty.MapVal(map[string]cty.Value{
-					"a": cty.ObjectVal(map[string]cty.Value{
-						"volume_type": cty.StringVal("gp2"),
-					}),
-				}),
 			}),
 			AfterValMarks: []cty.PathValueMarks{
 				{
@@ -3655,29 +3644,141 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
 					"id":  {Type: cty.String, Optional: true, Computed: true},
 					"ami": {Type: cty.String, Optional: true},
 				},
-				BlockTypes: map[string]*configschema.NestedBlock{
+			},
-					"root_block_device": {
+			ExpectedOutput: `  # test_instance.example will be created
-						Block: configschema.Block{
+  + resource "test_instance" "example" {
-							Attributes: map[string]*configschema.Attribute{
+      + ami = (sensitive)
-								"volume_type": {
+      + id  = "i-02ae66f368e8518a9"
-									Type:     cty.String,
+    }
-									Optional: true,
+`,
-									Computed: true,
+		},
-								},
+		"in-place update - before sensitive": {
-							},
+			Action: plans.Update,
-						},
+			Mode:   addrs.ManagedResourceMode,
-						Nesting: configschema.NestingMap,
+			Before: cty.ObjectVal(map[string]cty.Value{
-					},
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-BEFORE"),
+			}),
+			After: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-AFTER"),
+			}),
+			BeforeValMarks: []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},
 				},
 			},
 			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"
+		},
-        }
+		"in-place update - after sensitive, map type": {
+			Action: plans.Update,
+			Mode:   addrs.ManagedResourceMode,
+			Before: cty.ObjectVal(map[string]cty.Value{
+				"id": cty.StringVal("i-02ae66f368e8518a9"),
+				"tags": cty.MapVal(map[string]cty.Value{
+					"name": cty.StringVal("anna"),
+				}),
+			}),
+			After: cty.ObjectVal(map[string]cty.Value{
+				"id": cty.StringVal("i-02ae66f368e8518a9"),
+				"tags": cty.MapVal(map[string]cty.Value{
+					"name": cty.StringVal("bob"),
+				}),
+			}),
+			AfterValMarks: []cty.PathValueMarks{
+				{
+					Path:  cty.Path{cty.GetAttrStep{Name: "tags"}, cty.IndexStep{Key: cty.StringVal("name")}},
+					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},
+					"tags": {Type: cty.Map(cty.String), Optional: true},
+				},
+			},
+			ExpectedOutput: `  # test_instance.example will be updated in-place
+  ~ resource "test_instance" "example" {
+        id   = "i-02ae66f368e8518a9"
+      ~ tags = (sensitive)
+    }
+`,
+		},
+		"in-place update - both sensitive": {
+			Action: plans.Update,
+			Mode:   addrs.ManagedResourceMode,
+			Before: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-BEFORE"),
+			}),
+			After: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-AFTER"),
+			}),
+			BeforeValMarks: []cty.PathValueMarks{
+				{
+					Path:  cty.Path{cty.GetAttrStep{Name: "ami"}},
+					Marks: cty.NewValueMarks("sensitive"),
+				}},
+			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},
+				},
+			},
+			ExpectedOutput: `  # test_instance.example will be updated in-place
+  ~ resource "test_instance" "example" {
+      ~ ami = (sensitive)
+        id  = "i-02ae66f368e8518a9"
+    }
+`,
+		},
+		"deletion": {
+			Action: plans.Delete,
+			Mode:   addrs.ManagedResourceMode,
+			Before: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-BEFORE"),
+			}),
+			After: cty.NullVal(cty.EmptyObject),
+			BeforeValMarks: []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},
+				},
+			},
+			ExpectedOutput: `  # test_instance.example will be destroyed
+  - resource "test_instance" "example" {
+      - ami = (sensitive)
+      - id  = "i-02ae66f368e8518a9" -> null
     }
 `,
 		},

providers/provider.go

@@ -238,7 +238,7 @@ type PlanResourceChangeResponse struct {
 	// configuration is applied.
 	PlannedState cty.Value
 
-	// RequiresReplace is the list of thee attributes that are requiring
+	// RequiresReplace is the list of the attributes that are requiring
 	// resource replacement.
 	RequiresReplace []cty.Path
 

states/instance_object.go

@@ -98,10 +98,11 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res
 	// and raise an error about that.
 	val := cty.UnknownAsNull(o.Value)
 
-	// If it contains marks, dump those now
+	// If it contains marks, save these in state
 	unmarked := val
+	var pvm []cty.PathValueMarks
 	if val.ContainsMarked() {
-		unmarked, _ = val.UnmarkDeep()
+		unmarked, pvm = val.UnmarkDeepWithPaths()
 	}
 	src, err := ctyjson.Marshal(unmarked, ty)
 	if err != nil {
@@ -111,6 +112,7 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res
 	return &ResourceInstanceObjectSrc{
 		SchemaVersion:       schemaVersion,
 		AttrsJSON:           src,
+		AttrSensitivePaths:  pvm,
 		Private:             o.Private,
 		Status:              o.Status,
 		Dependencies:        o.Dependencies,

states/instance_object_src.go

@@ -49,6 +49,10 @@ type ResourceInstanceObjectSrc struct {
 	// the recommendations in the AttrsJSON documentation above.
 	AttrsFlat map[string]string
 
+	// AttrSensitivePaths is an array of paths to mark as sensitive coming out of
+	// state, or to save as sensitive paths when saving state
+	AttrSensitivePaths []cty.PathValueMarks
+
 	// These fields all correspond to the fields of the same name on
 	// ResourceInstanceObject.
 	Private             []byte
@@ -78,6 +82,10 @@ func (os *ResourceInstanceObjectSrc) Decode(ty cty.Type) (*ResourceInstanceObjec
 		}
 	} else {
 		val, err = ctyjson.Unmarshal(os.AttrsJSON, ty)
+		// Mark the value with paths if applicable
+		if os.AttrSensitivePaths != nil {
+			val = val.MarkWithPaths(os.AttrSensitivePaths)
+		}
 		if err != nil {
 			return nil, err
 		}

states/state_deepcopy.go

@@ -144,6 +144,12 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc {
 		copy(attrsJSON, obj.AttrsJSON)
 	}
 
+	var attrPaths []cty.PathValueMarks
+	if obj.AttrSensitivePaths != nil {
+		attrPaths = make([]cty.PathValueMarks, len(obj.AttrSensitivePaths))
+		copy(attrPaths, obj.AttrSensitivePaths)
+	}
+
 	var private []byte
 	if obj.Private != nil {
 		private = make([]byte, len(obj.Private))
@@ -164,6 +170,7 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc {
 		Private:             private,
 		AttrsFlat:           attrsFlat,
 		AttrsJSON:           attrsJSON,
+		AttrSensitivePaths:  attrPaths,
 		Dependencies:        dependencies,
 		CreateBeforeDestroy: obj.CreateBeforeDestroy,
 	}

states/state_test.go

@@ -230,7 +230,14 @@ func TestStateDeepCopy(t *testing.T) {
 			Status:        ObjectReady,
 			SchemaVersion: 1,
 			AttrsJSON:     []byte(`{"woozles":"confuzles"}`),
-			Private:       []byte("private data"),
+			// Sensitive path at "woozles"
+			AttrSensitivePaths: []cty.PathValueMarks{
+				{
+					Path:  cty.Path{cty.GetAttrStep{Name: "woozles"}},
+					Marks: cty.NewValueMarks("sensitive"),
+				},
+			},
+			Private: []byte("private data"),
 			Dependencies: []addrs.ConfigResource{
 				{
 					Module: addrs.RootModule,

states/statefile/version4.go

@@ -7,6 +7,7 @@ import (
 	"sort"
 
 	version "github.com/hashicorp/go-version"
+	"github.com/zclconf/go-cty/cty"
 	ctyjson "github.com/zclconf/go-cty/cty/json"
 
 	"github.com/hashicorp/terraform/addrs"
@@ -151,6 +152,24 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) {
 				}
 			}
 
+			// Sensitive paths
+			if isV4.AttributeSensitivePaths != nil {
+				paths, pathsDiags := unmarshalPaths([]byte(isV4.AttributeSensitivePaths))
+				diags = diags.Append(pathsDiags)
+				if pathsDiags.HasErrors() {
+					continue
+				}
+
+				var pvm []cty.PathValueMarks
+				for _, path := range paths {
+					pvm = append(pvm, cty.PathValueMarks{
+						Path:  path,
+						Marks: cty.NewValueMarks("sensitive"),
+					})
+				}
+				obj.AttrSensitivePaths = pvm
+			}
+
 			{
 				// Status
 				raw := isV4.Status
@@ -452,16 +471,27 @@ func appendInstanceObjectStateV4(rs *states.Resource, is *states.ResourceInstanc
 		}
 	}
 
+	// Extract paths from path value marks
+	var paths []cty.Path
+	for _, vm := range obj.AttrSensitivePaths {
+		paths = append(paths, vm.Path)
+	}
+
+	// Marshal paths to JSON
+	attributeSensitivePaths, pathsDiags := marshalPaths(paths)
+	diags = diags.Append(pathsDiags)
+
 	return append(isV4s, instanceObjectStateV4{
-		IndexKey:            rawKey,
+		IndexKey:                rawKey,
-		Deposed:             string(deposed),
+		Deposed:                 string(deposed),
-		Status:              status,
+		Status:                  status,
-		SchemaVersion:       obj.SchemaVersion,
+		SchemaVersion:           obj.SchemaVersion,
-		AttributesFlat:      obj.AttrsFlat,
+		AttributesFlat:          obj.AttrsFlat,
-		AttributesRaw:       obj.AttrsJSON,
+		AttributesRaw:           obj.AttrsJSON,
-		PrivateRaw:          privateRaw,
+		AttributeSensitivePaths: attributeSensitivePaths,
-		Dependencies:        deps,
+		PrivateRaw:              privateRaw,
-		CreateBeforeDestroy: obj.CreateBeforeDestroy,
+		Dependencies:            deps,
+		CreateBeforeDestroy:     obj.CreateBeforeDestroy,
 	}), diags
 }
 
@@ -505,9 +535,10 @@ type instanceObjectStateV4 struct {
 	Status   string      `json:"status,omitempty"`
 	Deposed  string      `json:"deposed,omitempty"`
 
-	SchemaVersion  uint64            `json:"schema_version"`
+	SchemaVersion           uint64            `json:"schema_version"`
-	AttributesRaw  json.RawMessage   `json:"attributes,omitempty"`
+	AttributesRaw           json.RawMessage   `json:"attributes,omitempty"`
-	AttributesFlat map[string]string `json:"attributes_flat,omitempty"`
+	AttributesFlat          map[string]string `json:"attributes_flat,omitempty"`
+	AttributeSensitivePaths json.RawMessage   `json:"sensitive_attributes,omitempty,"`
 
 	PrivateRaw []byte `json:"private,omitempty"`
 
@@ -577,3 +608,135 @@ func (si sortInstancesV4) Less(i, j int) bool {
 	}
 	return false
 }
+
+// pathStep is an intermediate representation of a cty.PathStep to facilitate
+// consistent JSON serialization. The Value field can either be a cty.Value of
+// dynamic type (for index steps), or a string (for get attr steps).
+type pathStep struct {
+	Type  string          `json:"type"`
+	Value json.RawMessage `json:"value"`
+}
+
+const (
+	indexPathStepType   = "index"
+	getAttrPathStepType = "get_attr"
+)
+
+func unmarshalPaths(buf []byte) ([]cty.Path, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+	var jsonPaths [][]pathStep
+
+	err := json.Unmarshal(buf, &jsonPaths)
+	if err != nil {
+		diags = diags.Append(tfdiags.Sourceless(
+			tfdiags.Error,
+			"Error unmarshaling path steps",
+			err.Error(),
+		))
+	}
+
+	paths := make([]cty.Path, 0, len(jsonPaths))
+
+unmarshalOuter:
+	for _, jsonPath := range jsonPaths {
+		var path cty.Path
+		for _, jsonStep := range jsonPath {
+			switch jsonStep.Type {
+			case indexPathStepType:
+				key, err := ctyjson.Unmarshal(jsonStep.Value, cty.DynamicPseudoType)
+				if err != nil {
+					diags = diags.Append(tfdiags.Sourceless(
+						tfdiags.Error,
+						"Error unmarshaling path step",
+						fmt.Sprintf("Failed to unmarshal index step key: %s", err),
+					))
+					continue unmarshalOuter
+				}
+				path = append(path, cty.IndexStep{Key: key})
+			case getAttrPathStepType:
+				var name string
+				if err := json.Unmarshal(jsonStep.Value, &name); err != nil {
+					diags = diags.Append(tfdiags.Sourceless(
+						tfdiags.Error,
+						"Error unmarshaling path step",
+						fmt.Sprintf("Failed to unmarshal get attr step name: %s", err),
+					))
+					continue unmarshalOuter
+				}
+				path = append(path, cty.GetAttrStep{Name: name})
+			default:
+				diags = diags.Append(tfdiags.Sourceless(
+					tfdiags.Error,
+					"Unsupported path step",
+					fmt.Sprintf("Unsupported path step type %q", jsonStep.Type),
+				))
+				continue unmarshalOuter
+			}
+		}
+		paths = append(paths, path)
+	}
+
+	return paths, diags
+}
+
+func marshalPaths(paths []cty.Path) ([]byte, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+
+	// cty.Path is a slice of cty.PathSteps, so our representation of a slice
+	// of paths is a nested slice of our intermediate pathStep struct
+	jsonPaths := make([][]pathStep, 0, len(paths))
+
+marshalOuter:
+	for _, path := range paths {
+		jsonPath := make([]pathStep, 0, len(path))
+		for _, step := range path {
+			var jsonStep pathStep
+			switch s := step.(type) {
+			case cty.IndexStep:
+				key, err := ctyjson.Marshal(s.Key, cty.DynamicPseudoType)
+				if err != nil {
+					diags = diags.Append(tfdiags.Sourceless(
+						tfdiags.Error,
+						"Error marshaling path step",
+						fmt.Sprintf("Failed to marshal index step key %#v: %s", s.Key, err),
+					))
+					continue marshalOuter
+				}
+				jsonStep.Type = indexPathStepType
+				jsonStep.Value = key
+			case cty.GetAttrStep:
+				name, err := json.Marshal(s.Name)
+				if err != nil {
+					diags = diags.Append(tfdiags.Sourceless(
+						tfdiags.Error,
+						"Error marshaling path step",
+						fmt.Sprintf("Failed to marshal get attr step name %s: %s", s.Name, err),
+					))
+					continue marshalOuter
+				}
+				jsonStep.Type = getAttrPathStepType
+				jsonStep.Value = name
+			default:
+				diags = diags.Append(tfdiags.Sourceless(
+					tfdiags.Error,
+					"Unsupported path step",
+					fmt.Sprintf("Unsupported path step %#v (%t)", step, step),
+				))
+				continue marshalOuter
+			}
+			jsonPath = append(jsonPath, jsonStep)
+		}
+		jsonPaths = append(jsonPaths, jsonPath)
+	}
+
+	buf, err := json.Marshal(jsonPaths)
+	if err != nil {
+		diags = diags.Append(tfdiags.Sourceless(
+			tfdiags.Error,
+			"Error marshaling path steps",
+			fmt.Sprintf("Failed to marshal path steps: %s", err),
+		))
+	}
+
+	return buf, diags
+}

states/statefile/version4_test.go

@@ -2,7 +2,11 @@ package statefile
 
 import (
 	"sort"
+	"strings"
 	"testing"
+
+	"github.com/hashicorp/terraform/tfdiags"
+	"github.com/zclconf/go-cty/cty"
 )
 
 // This test verifies that modules are sorted before resources:
@@ -39,3 +43,216 @@ func TestVersion4_sort(t *testing.T) {
 		}
 	}
 }
+
+func TestVersion4_unmarshalPaths(t *testing.T) {
+	testCases := map[string]struct {
+		json  string
+		paths []cty.Path
+		diags []string
+	}{
+		"no paths": {
+			json:  `[]`,
+			paths: []cty.Path{},
+		},
+		"attribute path": {
+			json: `[
+  [
+    {
+      "type": "get_attr",
+	  "value": "password"
+    }
+  ]
+]`,
+			paths: []cty.Path{cty.GetAttrPath("password")},
+		},
+		"attribute and string index": {
+			json: `[
+  [
+    {
+      "type": "get_attr",
+	  "value": "triggers"
+    },
+    {
+      "type": "index",
+      "value": {
+        "value": "secret",
+		"type": "string"
+      }
+    }
+  ]
+]`,
+			paths: []cty.Path{cty.GetAttrPath("triggers").IndexString("secret")},
+		},
+		"attribute, number index, attribute": {
+			json: `[
+  [
+    {
+      "type": "get_attr",
+	  "value": "identities"
+    },
+    {
+      "type": "index",
+      "value": {
+        "value": 2,
+		"type": "number"
+      }
+    },
+    {
+      "type": "get_attr",
+      "value": "private_key"
+    }
+  ]
+]`,
+			paths: []cty.Path{cty.GetAttrPath("identities").IndexInt(2).GetAttr("private_key")},
+		},
+		"multiple paths": {
+			json: `[
+  [
+    {
+      "type": "get_attr",
+	  "value": "alpha"
+    }
+  ],
+  [
+    {
+      "type": "get_attr",
+	  "value": "beta"
+    }
+  ],
+  [
+    {
+      "type": "get_attr",
+	  "value": "gamma"
+    }
+  ]
+]`,
+			paths: []cty.Path{cty.GetAttrPath("alpha"), cty.GetAttrPath("beta"), cty.GetAttrPath("gamma")},
+		},
+		"errors": {
+			json: `[
+  [
+    {
+      "type": "get_attr",
+	  "value": 5
+    }
+  ],
+  [
+    {
+      "type": "index",
+	  "value": "test"
+    }
+  ],
+  [
+    {
+      "type": "invalid_type",
+	  "value": ["this is invalid too"]
+    }
+  ]
+]`,
+			paths: []cty.Path{},
+			diags: []string{
+				"Failed to unmarshal get attr step name",
+				"Failed to unmarshal index step key",
+				"Unsupported path step",
+			},
+		},
+		"one invalid path, others valid": {
+			json: `[
+  [
+    {
+      "type": "get_attr",
+	  "value": "alpha"
+    }
+  ],
+  [
+    {
+      "type": "invalid_type",
+	  "value": ["this is invalid too"]
+    }
+  ],
+  [
+    {
+      "type": "get_attr",
+	  "value": "gamma"
+    }
+  ]
+]`,
+			paths: []cty.Path{cty.GetAttrPath("alpha"), cty.GetAttrPath("gamma")},
+			diags: []string{"Unsupported path step"},
+		},
+		"invalid structure": {
+			json:  `{}`,
+			paths: []cty.Path{},
+			diags: []string{"Error unmarshaling path steps"},
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			paths, diags := unmarshalPaths([]byte(tc.json))
+
+			if len(tc.diags) == 0 {
+				if len(diags) != 0 {
+					t.Errorf("expected no diags, got: %#v", diags)
+				}
+			} else {
+				if got, want := len(diags), len(tc.diags); got != want {
+					t.Fatalf("got %d diags, want %d\n%s", got, want, diags.Err())
+				}
+				for i := range tc.diags {
+					got := tfdiags.Diagnostics{diags[i]}.Err().Error()
+					if !strings.Contains(got, tc.diags[i]) {
+						t.Errorf("expected diag %d to contain %q, but was:\n%s", i, tc.diags[i], got)
+					}
+				}
+			}
+
+			if len(paths) != len(tc.paths) {
+				t.Fatalf("got %d paths, want %d", len(paths), len(tc.paths))
+			}
+			for i, path := range paths {
+				if !path.Equals(tc.paths[i]) {
+					t.Errorf("wrong paths\n got: %#v\nwant: %#v", path, tc.paths[i])
+				}
+			}
+		})
+	}
+}
+
+func TestVersion4_marshalPaths(t *testing.T) {
+	testCases := map[string]struct {
+		paths []cty.Path
+		json  string
+	}{
+		"no paths": {
+			paths: []cty.Path{},
+			json:  `[]`,
+		},
+		"attribute path": {
+			paths: []cty.Path{cty.GetAttrPath("password")},
+			json:  `[[{"type":"get_attr","value":"password"}]]`,
+		},
+		"attribute, number index, attribute": {
+			paths: []cty.Path{cty.GetAttrPath("identities").IndexInt(2).GetAttr("private_key")},
+			json:  `[[{"type":"get_attr","value":"identities"},{"type":"index","value":{"value":2,"type":"number"}},{"type":"get_attr","value":"private_key"}]]`,
+		},
+		"multiple paths": {
+			paths: []cty.Path{cty.GetAttrPath("a"), cty.GetAttrPath("b"), cty.GetAttrPath("c")},
+			json:  `[[{"type":"get_attr","value":"a"}],[{"type":"get_attr","value":"b"}],[{"type":"get_attr","value":"c"}]]`,
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			json, diags := marshalPaths(tc.paths)
+
+			if len(diags) != 0 {
+				t.Fatalf("expected no diags, got: %#v", diags)
+			}
+
+			if got, want := string(json), tc.json; got != want {
+				t.Fatalf("wrong JSON output\n got: %s\nwant: %s\n", got, want)
+			}
+		})
+	}
+}

terraform/context_apply_test.go

@@ -11916,3 +11916,82 @@ resource "test_resource" "c" {
 		t.Fatalf("apply errors: %s", diags.Err())
 	}
 }
+
+func TestContext2Apply_variableSensitivity(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+terraform {
+	experiments = [sensitive_variables]
+}
+
+variable "sensitive_var" {
+	default = "foo"
+	sensitive = true
+}
+
+resource "test_resource" "foo" {
+	value   = var.sensitive_var
+}`,
+	})
+
+	p := testProvider("test")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	if _, diags := ctx.Plan(); diags.HasErrors() {
+		t.Fatalf("plan errors: %s", diags.Err())
+	}
+
+	state, diags := ctx.Apply()
+	if diags.HasErrors() {
+		t.Fatalf("apply errors: %s", diags.Err())
+	}
+
+	// Run a second apply with no changes
+	ctx = testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+		State: state,
+	})
+
+	if _, diags := ctx.Plan(); diags.HasErrors() {
+		t.Fatalf("plan errors: %s", diags.Err())
+	}
+
+	state, diags = ctx.Apply()
+	if diags.HasErrors() {
+		t.Fatalf("apply errors: %s", diags.Err())
+	}
+
+	// Now change the variable value
+	ctx = testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+		Variables: InputValues{
+			"sensitive_var": &InputValue{
+				Value: cty.StringVal("bar"),
+			},
+		},
+		State: state,
+	})
+
+	if _, diags := ctx.Plan(); diags.HasErrors() {
+		t.Fatalf("plan errors: %s", diags.Err())
+	}
+
+	_, diags = ctx.Apply()
+	if diags.HasErrors() {
+		t.Fatalf("apply errors: %s", diags.Err())
+	}
+}

terraform/eval_apply.go

@@ -105,21 +105,28 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 
 	log.Printf("[DEBUG] %s: applying the planned %s change", n.Addr.Absolute(ctx.Path()), change.Action)
 
-	// If our config or After value contain any marked values,
+	// If our config, Before or After value contain any marked values,
 	// ensure those are stripped out before sending
 	// this to the provider
 	unmarkedConfigVal := configVal
 	if configVal.ContainsMarked() {
 		unmarkedConfigVal, _ = configVal.UnmarkDeep()
 	}
+
+	unmarkedBefore := change.Before
+	if change.Before.ContainsMarked() {
+		unmarkedBefore, _ = change.Before.UnmarkDeep()
+	}
+
 	unmarkedAfter := change.After
+	var afterPaths []cty.PathValueMarks
 	if change.After.ContainsMarked() {
-		unmarkedAfter, _ = change.After.UnmarkDeep()
+		unmarkedAfter, afterPaths = change.After.UnmarkDeepWithPaths()
 	}
 
 	resp := provider.ApplyResourceChange(providers.ApplyResourceChangeRequest{
 		TypeName:       n.Addr.Resource.Type,
-		PriorState:     change.Before,
+		PriorState:     unmarkedBefore,
 		Config:         unmarkedConfigVal,
 		PlannedState:   unmarkedAfter,
 		PlannedPrivate: change.Private,
@@ -138,6 +145,11 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 	// incomplete.
 	newVal := resp.NewState
 
+	// If we have paths to mark, mark those on this new value
+	if len(afterPaths) > 0 {
+		newVal = newVal.MarkWithPaths(afterPaths)
+	}
+
 	if newVal == cty.NilVal {
 		// Providers are supposed to return a partial new value even when errors
 		// occur, but sometimes they don't and so in that case we'll patch that up

terraform/eval_diff.go

@@ -213,7 +213,14 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		priorVal = cty.NullVal(schema.ImpliedType())
 	}
 
-	proposedNewVal := objchange.ProposedNewObject(schema, priorVal, unmarkedConfigVal)
+	unmarkedPriorVal := priorVal
+	if priorVal.ContainsMarked() {
+		// store the marked values so we can re-mark them later after
+		// we've sent things over the wire.
+		unmarkedPriorVal, _ = priorVal.UnmarkDeep()
+	}
+
+	proposedNewVal := objchange.ProposedNewObject(schema, unmarkedPriorVal, unmarkedConfigVal)
 
 	// Call pre-diff hook
 	if !n.Stub {
@@ -244,7 +251,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 	// we send back this information, we need to process ignore_changes
 	// so that CustomizeDiff will not act on them
 	var ignoreChangeDiags tfdiags.Diagnostics
-	proposedNewVal, ignoreChangeDiags = n.processIgnoreChanges(priorVal, proposedNewVal)
+	proposedNewVal, ignoreChangeDiags = n.processIgnoreChanges(unmarkedPriorVal, proposedNewVal)
 	diags = diags.Append(ignoreChangeDiags)
 	if ignoreChangeDiags.HasErrors() {
 		return nil, diags.Err()
@@ -253,7 +260,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 	resp := provider.PlanResourceChange(providers.PlanResourceChangeRequest{
 		TypeName:         n.Addr.Resource.Type,
 		Config:           unmarkedConfigVal,
-		PriorState:       priorVal,
+		PriorState:       unmarkedPriorVal,
 		ProposedNewState: proposedNewVal,
 		PriorPrivate:     priorPrivate,
 		ProviderMeta:     metaConfigVal,
@@ -274,7 +281,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 	}
 
 	// Add the marks back to the planned new value
-	if configVal.ContainsMarked() {
+	if len(unmarkedPaths) > 0 {
 		plannedNewVal = plannedNewVal.MarkWithPaths(unmarkedPaths)
 	}
 
@@ -352,7 +359,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 				continue
 			}
 
-			priorChangedVal, priorPathDiags := hcl.ApplyPath(priorVal, path, nil)
+			priorChangedVal, priorPathDiags := hcl.ApplyPath(unmarkedPriorVal, path, nil)
 			plannedChangedVal, plannedPathDiags := hcl.ApplyPath(plannedNewVal, path, nil)
 			if plannedPathDiags.HasErrors() && priorPathDiags.HasErrors() {
 				// This means the path was invalid in both the prior and new
@@ -384,7 +391,10 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 				plannedChangedVal = cty.NullVal(priorChangedVal.Type())
 			}
 
-			eqV := plannedChangedVal.Equals(priorChangedVal)
+			// Unmark for this value for the equality test. If only sensitivity has changed,
+			// this does not require an Update or Replace
+			unmarkedPlannedChangedVal, _ := plannedChangedVal.UnmarkDeep()
+			eqV := unmarkedPlannedChangedVal.Equals(priorChangedVal)
 			if !eqV.IsKnown() || eqV.False() {
 				reqRep.Add(path)
 			}
@@ -394,7 +404,10 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		}
 	}
 
-	eqV := plannedNewVal.Equals(priorVal)
+	// Unmark for this test for equality. If only sensitivity has changed,
+	// this does not require an Update or Replace
+	unmarkedPlannedNewVal, _ := plannedNewVal.UnmarkDeep()
+	eqV := unmarkedPlannedNewVal.Equals(unmarkedPriorVal)
 	eq := eqV.IsKnown() && eqV.True()
 
 	var action plans.Action
@@ -432,11 +445,11 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		nullPriorVal := cty.NullVal(schema.ImpliedType())
 
 		// create a new proposed value from the null state and the config
-		proposedNewVal = objchange.ProposedNewObject(schema, nullPriorVal, configVal)
+		proposedNewVal = objchange.ProposedNewObject(schema, nullPriorVal, unmarkedConfigVal)
 
 		resp = provider.PlanResourceChange(providers.PlanResourceChangeRequest{
 			TypeName:         n.Addr.Resource.Type,
-			Config:           configVal,
+			Config:           unmarkedConfigVal,
 			PriorState:       nullPriorVal,
 			ProposedNewState: proposedNewVal,
 			PriorPrivate:     plannedPrivate,
@@ -453,6 +466,11 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		}
 		plannedNewVal = resp.PlannedState
 		plannedPrivate = resp.PlannedPrivate
+
+		if len(unmarkedPaths) > 0 {
+			plannedNewVal = plannedNewVal.MarkWithPaths(unmarkedPaths)
+		}
+
 		for _, err := range plannedNewVal.Type().TestConformance(schema.ImpliedType()) {
 			diags = diags.Append(tfdiags.Sourceless(
 				tfdiags.Error,

terraform/eval_refresh.go

@@ -80,6 +80,13 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) {
 
 	// Refresh!
 	priorVal := state.Value
+
+	// Unmarked before sending to provider
+	var priorPaths []cty.PathValueMarks
+	if priorVal.ContainsMarked() {
+		priorVal, priorPaths = priorVal.UnmarkDeepWithPaths()
+	}
+
 	req := providers.ReadResourceRequest{
 		TypeName:     n.Addr.Resource.Type,
 		PriorState:   priorVal,
@@ -129,6 +136,11 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, err
 	}
 
+	// Mark the value if necessary
+	if len(priorPaths) > 0 {
+		newState.Value = newState.Value.MarkWithPaths(priorPaths)
+	}
+
 	if n.Output != nil {
 		*n.Output = newState
 	}