From 864b88d0ed3b72642939b8d6eac09d8da9567e24 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Thu, 18 Jan 2024 14:25:56 -0500
Subject: [PATCH] convert: prevent invalid types in dynamicReplace

When converting an unknown map into an object with optional attributes,
that object may have attributes which don't match the map element type.
This conversion will be valid, but when building Null or Unknown values
of that type we need to ensure all attributes have a valid type.

The fallthrough path for dynamicReplace when non-primitive types didn't
match contained a cty.NilVal element type, which would result in an
overall invalid type. We can early returns to ensure there are no
uninitialized values, and let the fallback use the original out type.
Since at this point the types appear to not be convertible, we can
assume they are from attributes which were skipped during conversion due
to being optional, otherwise the conversion would not have been valid.
---
 cty/convert/conversion_dynamic.go | 29 ++++++++++++++---------------
 cty/convert/public_test.go        | 24 ++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/cty/convert/conversion_dynamic.go b/cty/convert/conversion_dynamic.go
index 3b554e01..95f3925b 100644
--- a/cty/convert/conversion_dynamic.go
+++ b/cty/convert/conversion_dynamic.go
@@ -40,6 +40,11 @@ func dynamicPassthrough(in cty.Value, path cty.Path) (cty.Value, error) {
 // perspective, and will panic if it finds that they are not. For example if
 // in is an object and out is a map, this function will still attempt to iterate
 // through both as if they were the same.
+// While the outermost in and out types may be compatible from a Convert
+// perspective, inner types may not match when converting between maps and
+// objects with optional attributes when the optional attributes don't match
+// the map element type. Therefor in the case of a non-primitive type mismatch,
+// we have to assume conversion was possible and pass the out type through.
 func dynamicReplace(in, out cty.Type) cty.Type {
 	if in == cty.DynamicPseudoType || in == cty.NilType {
 		// Short circuit this case, there's no point worrying about this if in
@@ -56,11 +61,9 @@ func dynamicReplace(in, out cty.Type) cty.Type {
 		// return it unchanged.
 		return out
 	case out.IsMapType():
-		var elemType cty.Type
-
 		// Maps are compatible with other maps or objects.
 		if in.IsMapType() {
-			elemType = dynamicReplace(in.ElementType(), out.ElementType())
+			return cty.Map(dynamicReplace(in.ElementType(), out.ElementType()))
 		}
 
 		if in.IsObjectType() {
@@ -69,10 +72,10 @@ func dynamicReplace(in, out cty.Type) cty.Type {
 				types = append(types, t)
 			}
 			unifiedType, _ := unify(types, true)
-			elemType = dynamicReplace(unifiedType, out.ElementType())
+			return cty.Map(dynamicReplace(unifiedType, out.ElementType()))
 		}
 
-		return cty.Map(elemType)
+		return out
 	case out.IsObjectType():
 		// Objects are compatible with other objects and maps.
 		outTypes := map[string]cty.Type{}
@@ -97,33 +100,29 @@ func dynamicReplace(in, out cty.Type) cty.Type {
 
 		return cty.Object(outTypes)
 	case out.IsSetType():
-		var elemType cty.Type
-
 		// Sets are compatible with other sets, lists, tuples.
 		if in.IsSetType() || in.IsListType() {
-			elemType = dynamicReplace(in.ElementType(), out.ElementType())
+			return cty.Set(dynamicReplace(in.ElementType(), out.ElementType()))
 		}
 
 		if in.IsTupleType() {
 			unifiedType, _ := unify(in.TupleElementTypes(), true)
-			elemType = dynamicReplace(unifiedType, out.ElementType())
+			return cty.Set(dynamicReplace(unifiedType, out.ElementType()))
 		}
 
-		return cty.Set(elemType)
+		return out
 	case out.IsListType():
-		var elemType cty.Type
-
 		// Lists are compatible with other lists, sets, and tuples.
 		if in.IsSetType() || in.IsListType() {
-			elemType = dynamicReplace(in.ElementType(), out.ElementType())
+			return cty.List(dynamicReplace(in.ElementType(), out.ElementType()))
 		}
 
 		if in.IsTupleType() {
 			unifiedType, _ := unify(in.TupleElementTypes(), true)
-			elemType = dynamicReplace(unifiedType, out.ElementType())
+			return cty.List(dynamicReplace(unifiedType, out.ElementType()))
 		}
 
-		return cty.List(elemType)
+		return out
 	case out.IsTupleType():
 		// Tuples are only compatible with other tuples
 		var types []cty.Type
diff --git a/cty/convert/public_test.go b/cty/convert/public_test.go
index 85e92195..dc342a14 100644
--- a/cty/convert/public_test.go
+++ b/cty/convert/public_test.go
@@ -1770,6 +1770,30 @@ func TestConvert(t *testing.T) {
 			Type:  cty.String,
 			Want:  cty.UnknownVal(cty.String).RefineNotNull(),
 		},
+
+		// Make sure we get valid unknown attribute types when converting from
+		// a map to an object with optional attributes.
+		{
+			Value: cty.ObjectVal(map[string]cty.Value{
+				"TTTattr": cty.UnknownVal(cty.Map(cty.String)),
+			}),
+			Type: cty.Object(map[string]cty.Type{
+				"TTTattr": cty.ObjectWithOptionalAttrs(map[string]cty.Type{
+					"string": cty.String,
+					"set":    cty.Set(cty.String),
+					"list":   cty.List(cty.String),
+					"map":    cty.Map(cty.String),
+				}, []string{"set", "list", "map"}),
+			}),
+			Want: cty.ObjectVal(map[string]cty.Value{
+				"TTTattr": cty.UnknownVal(cty.Object(map[string]cty.Type{
+					"list":   cty.List(cty.String),
+					"map":    cty.Map(cty.String),
+					"set":    cty.Set(cty.String),
+					"string": cty.String,
+				})),
+			}),
+		},
 	}
 
 	for _, test := range tests {