From e9c1706fb0a45e1d00af6e21af1e5e7b8dc5f9df Mon Sep 17 00:00:00 2001
From: Alisdair McDiarmid <alisdair@users.noreply.github.com>
Date: Tue, 6 Oct 2020 10:53:43 -0400
Subject: [PATCH] cty: Fix path array reuse with UnmarkDeepWithPaths

When unmarking a complex value and retaining the marked paths, a
sufficiently deep structure would result in incorrect duplicate path
output. For example, this structure (in HCL syntax):

{
  environment = [
    {
      variables = {
        "x" = 1
        "y" = 2
      }
    }
  ]
}

If the 1 and 2 values are marked, the resulting path value marks from
UnmarkDeepWithPaths would have two entries, as expected. However, both
would have the same Path attribute, like so:

[
  { environment[0].variables["x"], "mark" },
  { environment[0].variables["x"], "mark" },
]

This is caused by calling `append` in the walk transform function with
the same path object repeatedly, which eventually does not result in
array reallocation, and therefore the path object is modified in-place.
---
 cty/marks.go      |  4 +++-
 cty/marks_test.go | 26 ++++++++++++++++++++++++++
 cty/walk.go       |  3 +++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/cty/marks.go b/cty/marks.go
index e3eaec3b..4e1d3b12 100644
--- a/cty/marks.go
+++ b/cty/marks.go
@@ -240,7 +240,9 @@ type unmarkTransformer struct {
 func (t *unmarkTransformer) Enter(p Path, v Value) (Value, error) {
 	unmarkedVal, marks := v.Unmark()
 	if len(marks) > 0 {
-		t.pvm = append(t.pvm, PathValueMarks{p, marks})
+		path := make(Path, len(p), len(p)+1)
+		copy(path, p)
+		t.pvm = append(t.pvm, PathValueMarks{path, marks})
 	}
 	return unmarkedVal, nil
 }
diff --git a/cty/marks_test.go b/cty/marks_test.go
index d7275e82..71775573 100644
--- a/cty/marks_test.go
+++ b/cty/marks_test.go
@@ -422,6 +422,32 @@ func TestPathValueMarks(t *testing.T) {
 				{GetAttrPath("z"), NewValueMarks("f")},
 			},
 		},
+		"path array reuse regression test": {
+			ObjectVal(map[string]Value{
+				"environment": ListVal([]Value{
+					ObjectVal(map[string]Value{
+						"variables": MapVal(map[string]Value{
+							"bar": StringVal("secret").Mark("sensitive"),
+							"foo": StringVal("secret").Mark("sensitive"),
+						}),
+					}),
+				}),
+			}),
+			ObjectVal(map[string]Value{
+				"environment": ListVal([]Value{
+					ObjectVal(map[string]Value{
+						"variables": MapVal(map[string]Value{
+							"bar": StringVal("secret"),
+							"foo": StringVal("secret"),
+						}),
+					}),
+				}),
+			}),
+			[]PathValueMarks{
+				{GetAttrPath("environment").IndexInt(0).GetAttr("variables").IndexString("bar"), NewValueMarks("sensitive")},
+				{GetAttrPath("environment").IndexInt(0).GetAttr("variables").IndexString("foo"), NewValueMarks("sensitive")},
+			},
+		},
 	}
 
 	for name, tc := range testCases {
diff --git a/cty/walk.go b/cty/walk.go
index 921d9539..d17f48cc 100644
--- a/cty/walk.go
+++ b/cty/walk.go
@@ -69,6 +69,9 @@ func walk(path Path, val Value, cb func(Path, Value) (bool, error)) error {
 // Use Enter when you want to transform a complex value before traversal
 // (preorder), and Exit when you want to transform a value after traversal
 // (postorder).
+//
+// The path passed to the given function may not be used after that function
+// returns, since its backing array is re-used for other calls.
 type Transformer interface {
 	Enter(Path, Value) (Value, error)
 	Exit(Path, Value) (Value, error)