configs: Add sensitive marks for nested attributes

Object values returned from providers have their attributes marked as
sensitive based on the provider schema. This was not fully implemented
for nested attribute types, which is corrected in this commit.

internal/configs/configschema/implied_type.go

@@ -44,6 +44,9 @@ func (b *Block) ContainsSensitive() bool {
 		if attrS.Sensitive {
 			return true
 		}
+		if attrS.NestedType != nil && attrS.NestedType.ContainsSensitive() {
+			return true
+		}
 	}
 	for _, blockS := range b.BlockTypes {
 		if blockS.ContainsSensitive() {
@@ -108,8 +111,8 @@ func (o *Object) ContainsSensitive() bool {
 		if attrS.Sensitive {
 			return true
 		}
-		if attrS.NestedType != nil {
+		if attrS.NestedType != nil && attrS.NestedType.ContainsSensitive() {
-			return attrS.NestedType.ContainsSensitive()
+			return true
 		}
 	}
 	return false

internal/configs/configschema/implied_type_test.go

@@ -154,6 +154,70 @@ func TestBlockImpliedType(t *testing.T) {
 	}
 }
 
+func TestBlockContainsSensitive(t *testing.T) {
+	tests := map[string]struct {
+		Schema *Block
+		Want   bool
+	}{
+		"object contains sensitive": {
+			&Block{
+				Attributes: map[string]*Attribute{
+					"sensitive": {Sensitive: true},
+				},
+			},
+			true,
+		},
+		"no sensitive attrs": {
+			&Block{
+				Attributes: map[string]*Attribute{
+					"insensitive": {},
+				},
+			},
+			false,
+		},
+		"nested object contains sensitive": {
+			&Block{
+				Attributes: map[string]*Attribute{
+					"nested": {
+						NestedType: &Object{
+							Nesting: NestingSingle,
+							Attributes: map[string]*Attribute{
+								"sensitive": {Sensitive: true},
+							},
+						},
+					},
+				},
+			},
+			true,
+		},
+		"nested obj, no sensitive attrs": {
+			&Block{
+				Attributes: map[string]*Attribute{
+					"nested": {
+						NestedType: &Object{
+							Nesting: NestingSingle,
+							Attributes: map[string]*Attribute{
+								"public": {},
+							},
+						},
+					},
+				},
+			},
+			false,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			got := test.Schema.ContainsSensitive()
+			if got != test.Want {
+				t.Errorf("wrong result\ngot:  %#v\nwant: %#v", got, test.Want)
+			}
+		})
+	}
+
+}
+
 func TestObjectImpliedType(t *testing.T) {
 	tests := map[string]struct {
 		Schema *Object
@@ -353,6 +417,37 @@ func TestObjectContainsSensitive(t *testing.T) {
 			},
 			false,
 		},
+		"several nested objects, one contains sensitive": {
+			&Object{
+				Attributes: map[string]*Attribute{
+					"alpha": {
+						NestedType: &Object{
+							Nesting: NestingSingle,
+							Attributes: map[string]*Attribute{
+								"nonsensitive": {},
+							},
+						},
+					},
+					"beta": {
+						NestedType: &Object{
+							Nesting: NestingSingle,
+							Attributes: map[string]*Attribute{
+								"sensitive": {Sensitive: true},
+							},
+						},
+					},
+					"gamma": {
+						NestedType: &Object{
+							Nesting: NestingSingle,
+							Attributes: map[string]*Attribute{
+								"nonsensitive": {},
+							},
+						},
+					},
+				},
+			},
+			true,
+		},
 	}
 
 	for name, test := range tests {

internal/configs/configschema/marks.go

@@ -12,6 +12,8 @@ import (
 // blocks are descended (if present in the given value).
 func (b *Block) ValueMarks(val cty.Value, path cty.Path) []cty.PathValueMarks {
 	var pvm []cty.PathValueMarks
+
+	// We can mark attributes as sensitive even if the value is null
 	for name, attrS := range b.Attributes {
 		if attrS.Sensitive {
 			// Create a copy of the path, with this step added, to add to our PathValueMarks slice
@@ -25,9 +27,28 @@ func (b *Block) ValueMarks(val cty.Value, path cty.Path) []cty.PathValueMarks {
 		}
 	}
 
+	// If the value is null, no other marks are possible
 	if val.IsNull() {
 		return pvm
 	}
+
+	// Extract marks for nested attribute type values
+	for name, attrS := range b.Attributes {
+		// If the attribute has no nested type, or the nested type doesn't
+		// contain any sensitive attributes, skip inspecting it
+		if attrS.NestedType == nil || !attrS.NestedType.ContainsSensitive() {
+			continue
+		}
+
+		// Create a copy of the path, with this step added, to add to our PathValueMarks slice
+		attrPath := make(cty.Path, len(path), len(path)+1)
+		copy(attrPath, path)
+		attrPath = append(path, cty.GetAttrStep{Name: name})
+
+		pvm = append(pvm, attrS.NestedType.ValueMarks(val.GetAttr(name), attrPath)...)
+	}
+
+	// Extract marks for nested blocks
 	for name, blockS := range b.BlockTypes {
 		// If our block doesn't contain any sensitive attributes, skip inspecting it
 		if !blockS.Block.ContainsSensitive() {
@@ -59,3 +80,72 @@ func (b *Block) ValueMarks(val cty.Value, path cty.Path) []cty.PathValueMarks {
 	}
 	return pvm
 }
+
+// ValueMarks returns a set of path value marks for a given value and path,
+// based on the sensitive flag for each attribute within the nested attribute.
+// Attributes with nested types are descended (if present in the given value).
+func (o *Object) ValueMarks(val cty.Value, path cty.Path) []cty.PathValueMarks {
+	var pvm []cty.PathValueMarks
+
+	if val.IsNull() || !val.IsKnown() {
+		return pvm
+	}
+
+	for name, attrS := range o.Attributes {
+		// Skip attributes which can never produce sensitive path value marks
+		if !attrS.Sensitive && (attrS.NestedType == nil || !attrS.NestedType.ContainsSensitive()) {
+			continue
+		}
+
+		switch o.Nesting {
+		case NestingSingle, NestingGroup:
+			// Create a path to this attribute
+			attrPath := make(cty.Path, len(path), len(path)+1)
+			copy(attrPath, path)
+			attrPath = append(path, cty.GetAttrStep{Name: name})
+
+			if attrS.Sensitive {
+				// If the entire attribute is sensitive, mark it so
+				pvm = append(pvm, cty.PathValueMarks{
+					Path:  attrPath,
+					Marks: cty.NewValueMarks(marks.Sensitive),
+				})
+			} else {
+				// The attribute has a nested type which contains sensitive
+				// attributes, so recurse
+				pvm = append(pvm, attrS.NestedType.ValueMarks(val.GetAttr(name), attrPath)...)
+			}
+		case NestingList, NestingMap, NestingSet:
+			// For nested attribute types which have a non-single nesting mode,
+			// we add path value marks for each element of the collection
+			for it := val.ElementIterator(); it.Next(); {
+				idx, attrEV := it.Element()
+				attrV := attrEV.GetAttr(name)
+
+				// Create a path to this element of the attribute's collection. Note
+				// that the path is extended in opposite order to the iteration order
+				// of the loops: index into the collection, then the contained
+				// attribute name. This is because we have one type
+				// representing multiple collection elements.
+				attrPath := make(cty.Path, len(path), len(path)+2)
+				copy(attrPath, path)
+				attrPath = append(path, cty.IndexStep{Key: idx}, cty.GetAttrStep{Name: name})
+
+				if attrS.Sensitive {
+					// If the entire attribute is sensitive, mark it so
+					pvm = append(pvm, cty.PathValueMarks{
+						Path:  attrPath,
+						Marks: cty.NewValueMarks(marks.Sensitive),
+					})
+				} else {
+					// The attribute has a nested type which contains sensitive
+					// attributes, so recurse
+					pvm = append(pvm, attrS.NestedType.ValueMarks(attrV, attrPath)...)
+				}
+			}
+		default:
+			panic(fmt.Sprintf("unsupported nesting mode %s", attrS.NestedType.Nesting))
+		}
+	}
+	return pvm
+}

internal/configs/configschema/marks_test.go

@@ -1,7 +1,6 @@
 package configschema
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/hashicorp/terraform/internal/lang/marks"
@@ -19,6 +18,20 @@ func TestBlockValueMarks(t *testing.T) {
 				Type:      cty.String,
 				Sensitive: true,
 			},
+			"nested": {
+				NestedType: &Object{
+					Attributes: map[string]*Attribute{
+						"boop": {
+							Type: cty.String,
+						},
+						"honk": {
+							Type:      cty.String,
+							Sensitive: true,
+						},
+					},
+					Nesting: NestingList,
+				},
+			},
 		},
 
 		BlockTypes: map[string]*NestedBlock{
@@ -40,34 +53,46 @@ func TestBlockValueMarks(t *testing.T) {
 		},
 	}
 
-	for _, tc := range []struct {
+	testCases := map[string]struct {
 		given  cty.Value
 		expect cty.Value
 	}{
-		{
+		"unknown object": {
 			cty.UnknownVal(schema.ImpliedType()),
 			cty.UnknownVal(schema.ImpliedType()),
 		},
-		{
+		"null object": {
 			cty.NullVal(schema.ImpliedType()),
 			cty.NullVal(schema.ImpliedType()),
 		},
-		{
+		"object with unknown attributes and blocks": {
 			cty.ObjectVal(map[string]cty.Value{
 				"sensitive":   cty.UnknownVal(cty.String),
 				"unsensitive": cty.UnknownVal(cty.String),
+				"nested": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"boop": cty.String,
+					"honk": cty.String,
+				}))),
 				"list": cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()),
 			}),
 			cty.ObjectVal(map[string]cty.Value{
 				"sensitive":   cty.UnknownVal(cty.String).Mark(marks.Sensitive),
 				"unsensitive": cty.UnknownVal(cty.String),
+				"nested": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"boop": cty.String,
+					"honk": cty.String,
+				}))),
 				"list": cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()),
 			}),
 		},
-		{
+		"object with block value": {
 			cty.ObjectVal(map[string]cty.Value{
 				"sensitive":   cty.NullVal(cty.String),
 				"unsensitive": cty.UnknownVal(cty.String),
+				"nested": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"boop": cty.String,
+					"honk": cty.String,
+				}))),
 				"list": cty.ListVal([]cty.Value{
 					cty.ObjectVal(map[string]cty.Value{
 						"sensitive":   cty.UnknownVal(cty.String),
@@ -82,6 +107,10 @@ func TestBlockValueMarks(t *testing.T) {
 			cty.ObjectVal(map[string]cty.Value{
 				"sensitive":   cty.NullVal(cty.String).Mark(marks.Sensitive),
 				"unsensitive": cty.UnknownVal(cty.String),
+				"nested": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"boop": cty.String,
+					"honk": cty.String,
+				}))),
 				"list": cty.ListVal([]cty.Value{
 					cty.ObjectVal(map[string]cty.Value{
 						"sensitive":   cty.UnknownVal(cty.String).Mark(marks.Sensitive),
@@ -94,8 +123,56 @@ func TestBlockValueMarks(t *testing.T) {
 				}),
 			}),
 		},
-	} {
+		"object with known values and nested attribute": {
-		t.Run(fmt.Sprintf("%#v", tc.given), func(t *testing.T) {
+			cty.ObjectVal(map[string]cty.Value{
+				"sensitive":   cty.StringVal("foo"),
+				"unsensitive": cty.StringVal("bar"),
+				"nested": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.StringVal("foo"),
+						"honk": cty.StringVal("bar"),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.NullVal(cty.String),
+						"honk": cty.NullVal(cty.String),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.UnknownVal(cty.String),
+						"honk": cty.UnknownVal(cty.String),
+					}),
+				}),
+				"list": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"sensitive":   cty.String,
+					"unsensitive": cty.String,
+				}))),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"sensitive":   cty.StringVal("foo").Mark(marks.Sensitive),
+				"unsensitive": cty.StringVal("bar"),
+				"nested": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.StringVal("foo"),
+						"honk": cty.StringVal("bar").Mark(marks.Sensitive),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.NullVal(cty.String),
+						"honk": cty.NullVal(cty.String).Mark(marks.Sensitive),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"boop": cty.UnknownVal(cty.String),
+						"honk": cty.UnknownVal(cty.String).Mark(marks.Sensitive),
+					}),
+				}),
+				"list": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{
+					"sensitive":   cty.String,
+					"unsensitive": cty.String,
+				}))),
+			}),
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
 			got := tc.given.MarkWithPaths(schema.ValueMarks(tc.given, nil))
 			if !got.RawEquals(tc.expect) {
 				t.Fatalf("\nexpected: %#v\ngot:      %#v\n", tc.expect, got)