From e0c6b3fcda9cc1766951bd935442215c91b098a2 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Fri, 7 May 2021 11:57:37 -0400 Subject: [PATCH 1/2] functions: Improve marks support for length Similar to cty's implementation, we only need to preserve marks from the value itself, not any nested values it may contain. This means that taking the length of an umarked list with marked elements results in an unmarked number. --- lang/funcs/collection.go | 8 +++--- lang/funcs/collection_test.go | 48 +++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/lang/funcs/collection.go b/lang/funcs/collection.go index ce6c8dc8f..0536dcfdf 100644 --- a/lang/funcs/collection.go +++ b/lang/funcs/collection.go @@ -20,6 +20,7 @@ var LengthFunc = function.New(&function.Spec{ Type: cty.DynamicPseudoType, AllowDynamicType: true, AllowUnknown: true, + AllowMarked: true, }, }, Type: func(args []cty.Value) (cty.Type, error) { @@ -34,15 +35,16 @@ var LengthFunc = function.New(&function.Spec{ Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) { coll := args[0] collTy := args[0].Type() + marks := coll.Marks() switch { case collTy == cty.DynamicPseudoType: - return cty.UnknownVal(cty.Number), nil + return cty.UnknownVal(cty.Number).WithMarks(marks), nil case collTy.IsTupleType(): l := len(collTy.TupleElementTypes()) - return cty.NumberIntVal(int64(l)), nil + return cty.NumberIntVal(int64(l)).WithMarks(marks), nil case collTy.IsObjectType(): l := len(collTy.AttributeTypes()) - return cty.NumberIntVal(int64(l)), nil + return cty.NumberIntVal(int64(l)).WithMarks(marks), nil case collTy == cty.String: // We'll delegate to the cty stdlib strlen function here, because // it deals with all of the complexities of tokenizing unicode diff --git a/lang/funcs/collection_test.go b/lang/funcs/collection_test.go index 9cc428a91..974a8437d 100644 --- a/lang/funcs/collection_test.go +++ b/lang/funcs/collection_test.go @@ -122,6 +122,54 @@ func TestLength(t *testing.T) { cty.DynamicVal, cty.UnknownVal(cty.Number), }, + { // Marked collections return a marked length + cty.ListVal([]cty.Value{ + cty.StringVal("hello"), + cty.StringVal("world"), + }).Mark("secret"), + cty.NumberIntVal(2).Mark("secret"), + }, + { // Marks on values in unmarked collections do not propagate + cty.ListVal([]cty.Value{ + cty.StringVal("hello").Mark("a"), + cty.StringVal("world").Mark("b"), + }), + cty.NumberIntVal(2), + }, + { // Marked strings return a marked length + cty.StringVal("hello world").Mark("secret"), + cty.NumberIntVal(11).Mark("secret"), + }, + { // Marked tuples return a marked length + cty.TupleVal([]cty.Value{ + cty.StringVal("hello"), + cty.StringVal("world"), + }).Mark("secret"), + cty.NumberIntVal(2).Mark("secret"), + }, + { // Marks on values in unmarked tuples do not propagate + cty.TupleVal([]cty.Value{ + cty.StringVal("hello").Mark("a"), + cty.StringVal("world").Mark("b"), + }), + cty.NumberIntVal(2), + }, + { // Marked objects return a marked length + cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("hello"), + "b": cty.StringVal("world"), + "c": cty.StringVal("nice to meet you"), + }).Mark("secret"), + cty.NumberIntVal(3).Mark("secret"), + }, + { // Marks on object attribute values do not propagate + cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("hello").Mark("a"), + "b": cty.StringVal("world").Mark("b"), + "c": cty.StringVal("nice to meet you").Mark("c"), + }), + cty.NumberIntVal(3), + }, } for _, test := range tests { From dbe5272931992727431adca3eaaf817368f1da23 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Fri, 7 May 2021 12:51:06 -0400 Subject: [PATCH 2/2] functions: Improve marks support for lookup Several changes to lookup to improve how we handle marked values: - If the entire collection is marked, preserve the marks on any result (whether successful or fallback) - If a returned value from the collection is marked, preserve the marks from only that value, combined with any overall collection marks - Retain marks on the fallback value when it is returned, combined with any overall collection marks - Include marks on the key in the result, as otherwise the result it ends up selecting could imply what the sensitive value was - Retain collection marks when returning an unknown value for a not wholly-known collection See also https://github.com/zclconf/go-cty/pull/98 --- lang/funcs/collection.go | 40 ++++++++++++----- lang/funcs/collection_test.go | 82 +++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 12 deletions(-) diff --git a/lang/funcs/collection.go b/lang/funcs/collection.go index 0536dcfdf..f9b3e6ae4 100644 --- a/lang/funcs/collection.go +++ b/lang/funcs/collection.go @@ -214,12 +214,14 @@ var IndexFunc = function.New(&function.Spec{ var LookupFunc = function.New(&function.Spec{ Params: []function.Parameter{ { - Name: "inputMap", - Type: cty.DynamicPseudoType, + Name: "inputMap", + Type: cty.DynamicPseudoType, + AllowMarked: true, }, { - Name: "key", - Type: cty.String, + Name: "key", + Type: cty.String, + AllowMarked: true, }, }, VarParam: &function.Parameter{ @@ -228,6 +230,7 @@ var LookupFunc = function.New(&function.Spec{ AllowUnknown: true, AllowDynamicType: true, AllowNull: true, + AllowMarked: true, }, Type: func(args []cty.Value) (ret cty.Type, err error) { if len(args) < 1 || len(args) > 3 { @@ -242,7 +245,8 @@ var LookupFunc = function.New(&function.Spec{ return cty.DynamicPseudoType, nil } - key := args[1].AsString() + keyVal, _ := args[1].Unmark() + key := keyVal.AsString() if ty.HasAttribute(key) { return args[0].GetAttr(key).Type(), nil } else if len(args) == 3 { @@ -268,23 +272,35 @@ var LookupFunc = function.New(&function.Spec{ defaultValueSet := false if len(args) == 3 { + // intentionally leave default value marked defaultVal = args[2] defaultValueSet = true } - mapVar := args[0] - lookupKey := args[1].AsString() + // keep track of marks from the collection and key + var markses []cty.ValueMarks + + // unmark collection, retain marks to reapply later + mapVar, mapMarks := args[0].Unmark() + markses = append(markses, mapMarks) + + // include marks on the key in the result + keyVal, keyMarks := args[1].Unmark() + if len(keyMarks) > 0 { + markses = append(markses, keyMarks) + } + lookupKey := keyVal.AsString() if !mapVar.IsKnown() { - return cty.UnknownVal(retType), nil + return cty.UnknownVal(retType).WithMarks(markses...), nil } if mapVar.Type().IsObjectType() { if mapVar.Type().HasAttribute(lookupKey) { - return mapVar.GetAttr(lookupKey), nil + return mapVar.GetAttr(lookupKey).WithMarks(markses...), nil } } else if mapVar.HasIndex(cty.StringVal(lookupKey)) == cty.True { - return mapVar.Index(cty.StringVal(lookupKey)), nil + return mapVar.Index(cty.StringVal(lookupKey)).WithMarks(markses...), nil } if defaultValueSet { @@ -292,10 +308,10 @@ var LookupFunc = function.New(&function.Spec{ if err != nil { return cty.NilVal, err } - return defaultVal, nil + return defaultVal.WithMarks(markses...), nil } - return cty.UnknownVal(cty.DynamicPseudoType), fmt.Errorf( + return cty.UnknownVal(cty.DynamicPseudoType).WithMarks(markses...), fmt.Errorf( "lookup failed to find '%s'", lookupKey) }, }) diff --git a/lang/funcs/collection_test.go b/lang/funcs/collection_test.go index 974a8437d..3ca3f9181 100644 --- a/lang/funcs/collection_test.go +++ b/lang/funcs/collection_test.go @@ -795,6 +795,88 @@ func TestLookup(t *testing.T) { cty.DynamicVal, // if the key is unknown then we don't know which object attribute and thus can't know the type false, }, + { // successful marked collection lookup returns marked value + []cty.Value{ + cty.MapVal(map[string]cty.Value{ + "boop": cty.StringVal("beep"), + }).Mark("a"), + cty.StringVal("boop"), + cty.StringVal("nope"), + }, + cty.StringVal("beep").Mark("a"), + false, + }, + { // apply collection marks to unknown return vaue + []cty.Value{ + cty.MapVal(map[string]cty.Value{ + "boop": cty.StringVal("beep"), + "frob": cty.UnknownVal(cty.String), + }).Mark("a"), + cty.StringVal("frob"), + cty.StringVal("nope"), + }, + cty.UnknownVal(cty.String).Mark("a"), + false, + }, + { // propagate collection marks to default when returning + []cty.Value{ + cty.MapVal(map[string]cty.Value{ + "boop": cty.StringVal("beep"), + }).Mark("a"), + cty.StringVal("frob"), + cty.StringVal("nope").Mark("b"), + }, + cty.StringVal("nope").WithMarks(cty.NewValueMarks("a", "b")), + false, + }, + { // on unmarked collection, return only marks from found value + []cty.Value{ + cty.MapVal(map[string]cty.Value{ + "boop": cty.StringVal("beep").Mark("a"), + "frob": cty.StringVal("honk").Mark("b"), + }), + cty.StringVal("frob"), + cty.StringVal("nope").Mark("c"), + }, + cty.StringVal("honk").Mark("b"), + false, + }, + { // on unmarked collection, return default exactly on missing + []cty.Value{ + cty.MapVal(map[string]cty.Value{ + "boop": cty.StringVal("beep").Mark("a"), + "frob": cty.StringVal("honk").Mark("b"), + }), + cty.StringVal("squish"), + cty.StringVal("nope").Mark("c"), + }, + cty.StringVal("nope").Mark("c"), + false, + }, + { // retain marks on default if converted + []cty.Value{ + cty.MapVal(map[string]cty.Value{ + "boop": cty.StringVal("beep").Mark("a"), + "frob": cty.StringVal("honk").Mark("b"), + }), + cty.StringVal("squish"), + cty.NumberIntVal(5).Mark("c"), + }, + cty.StringVal("5").Mark("c"), + false, + }, + { // propagate marks from key + []cty.Value{ + cty.MapVal(map[string]cty.Value{ + "boop": cty.StringVal("beep"), + "frob": cty.StringVal("honk"), + }), + cty.StringVal("boop").Mark("a"), + cty.StringVal("nope"), + }, + cty.StringVal("beep").Mark("a"), + false, + }, } for _, test := range tests {