From fbed52a353b3c91095ed6940d2f0a44f93924653 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 24 Nov 2021 17:30:42 -0500 Subject: [PATCH] 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 | 7 +- .../configs/configschema/implied_type_test.go | 95 ++++++++++++++++++ internal/configs/configschema/marks.go | 90 +++++++++++++++++ internal/configs/configschema/marks_test.go | 97 +++++++++++++++++-- 4 files changed, 277 insertions(+), 12 deletions(-) diff --git a/internal/configs/configschema/implied_type.go b/internal/configs/configschema/implied_type.go index 1a19a8c07..0a1dc75f9 100644 --- a/internal/configs/configschema/implied_type.go +++ b/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 { - return attrS.NestedType.ContainsSensitive() + if attrS.NestedType != nil && attrS.NestedType.ContainsSensitive() { + return true } } return false diff --git a/internal/configs/configschema/implied_type_test.go b/internal/configs/configschema/implied_type_test.go index 0fd8b01b5..6e3c0de72 100644 --- a/internal/configs/configschema/implied_type_test.go +++ b/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 { diff --git a/internal/configs/configschema/marks.go b/internal/configs/configschema/marks.go index aa07a41a1..c581b187b 100644 --- a/internal/configs/configschema/marks.go +++ b/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 +} diff --git a/internal/configs/configschema/marks_test.go b/internal/configs/configschema/marks_test.go index b4895ea91..2077e5e80 100644 --- a/internal/configs/configschema/marks_test.go +++ b/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), - "list": cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()), + "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), - "list": cty.UnknownVal(schema.BlockTypes["list"].ImpliedType()), + "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) { }), }), }, - } { - t.Run(fmt.Sprintf("%#v", tc.given), func(t *testing.T) { + "object with known values and nested attribute": { + 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)