From 0fee440edcce25699471433d3136bd24e5878fe2 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Wed, 12 Oct 2022 12:38:27 -0600 Subject: [PATCH] fix: don't reveal nested attributes with sensitive schema --- internal/command/format/diff.go | 21 +++- internal/command/format/diff_test.go | 158 +++++++++++++++++++++++---- 2 files changed, 152 insertions(+), 27 deletions(-) diff --git a/internal/command/format/diff.go b/internal/command/format/diff.go index 8c7d3ce84ba9..42e5785ee5f8 100644 --- a/internal/command/format/diff.go +++ b/internal/command/format/diff.go @@ -398,7 +398,7 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At } if attrS.NestedType != nil { - p.writeNestedAttrDiff(name, attrS.NestedType, old, new, nameLen, indent, path, action, showJustNew) + p.writeNestedAttrDiff(name, attrS, old, new, nameLen, indent, path, action, showJustNew) return false } @@ -441,9 +441,11 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At // writeNestedAttrDiff is responsible for formatting Attributes with NestedTypes // in the diff. func (p *blockBodyDiffPrinter) writeNestedAttrDiff( - name string, objS *configschema.Object, old, new cty.Value, + name string, attrWithNestedS *configschema.Attribute, old, new cty.Value, nameLen, indent int, path cty.Path, action plans.Action, showJustNew bool) { + objS := attrWithNestedS.NestedType + p.buf.WriteString("\n") p.writeSensitivityWarning(old, new, indent, action, false) p.buf.WriteString(strings.Repeat(" ", indent)) @@ -454,11 +456,22 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff( p.buf.WriteString(p.color.Color("[reset]")) p.buf.WriteString(strings.Repeat(" ", nameLen-len(name))) - if old.HasMark(marks.Sensitive) || new.HasMark(marks.Sensitive) { - p.buf.WriteString(" = (sensitive value)") + // Then schema of the attribute itself can be marked sensitive, or the values assigned + sensitive := attrWithNestedS.Sensitive || old.HasMark(marks.Sensitive) || new.HasMark(marks.Sensitive) + if sensitive { + p.buf.WriteString(" = (sensitive") + if attrWithNestedS.Sensitive { + p.buf.WriteRune(')') + } else { + p.buf.WriteString(" value)") + } if p.pathForcesNewResource(path) { p.buf.WriteString(p.color.Color(forcesNewResourceCaption)) } + + if new.IsNull() { + p.buf.WriteString(p.color.Color("[dark_gray] -> null[reset]")) + } return } diff --git a/internal/command/format/diff_test.go b/internal/command/format/diff_test.go index 56e2eafeafed..42beabc29b58 100644 --- a/internal/command/format/diff_test.go +++ b/internal/command/format/diff_test.go @@ -2164,6 +2164,58 @@ func TestResourceChange_primitiveSet(t *testing.T) { runTestCases(t, testCases) } +func TestResourceChange_nested_attributes(t *testing.T) { + testCases := map[string]testCase{ + "creation": { + Action: plans.Create, + Mode: addrs.ManagedResourceMode, + Before: cty.NullVal(cty.EmptyObject), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "nested_single": cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("hello"), + }), + "nested_list": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("hello"), + }), + }), + }), + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "nested_single": {NestedType: &configschema.Object{ + Attributes: map[string]*configschema.Attribute{ + "attr": {Type: cty.String, Optional: true}, + }, + Nesting: configschema.NestingSingle, + }, Optional: true}, + "nested_list": {NestedType: &configschema.Object{ + Attributes: map[string]*configschema.Attribute{ + "attr": {Type: cty.String, Optional: true}, + }, + Nesting: configschema.NestingList, + }}, + }, + }, + ExpectedOutput: ` # test_instance.example will be created + + resource "test_instance" "example" { + + id = (known after apply) + + nested_list = [ + + { + + attr = "hello" + }, + ] + + nested_single = { + + attr = "hello" + } + } +`, + }, + } + runTestCases(t, testCases) +} + func TestResourceChange_map(t *testing.T) { testCases := map[string]testCase{ "in-place update - creation": { @@ -4829,6 +4881,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "another": cty.StringVal("not secret"), }), }), + "nested_sensitive": cty.ObjectVal(map[string]cty.Value{ + "an_attr": cty.StringVal("secret because nested_sensitive is sensitive"), + }), }), AfterValMarks: []cty.PathValueMarks{ { @@ -4865,6 +4920,12 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "map_whole": {Type: cty.Map(cty.String), Optional: true}, "map_key": {Type: cty.Map(cty.Number), Optional: true}, "list_field": {Type: cty.List(cty.String), Optional: true}, + "nested_sensitive": {NestedType: &configschema.Object{ + Attributes: map[string]*configschema.Attribute{ + "an_attr": {Type: cty.String, Optional: true}, + }, + Nesting: configschema.NestingSingle, + }, Sensitive: true}, }, BlockTypes: map[string]*configschema.NestedBlock{ "nested_block_list": { @@ -4889,18 +4950,19 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { }, ExpectedOutput: ` # test_instance.example will be created + resource "test_instance" "example" { - + ami = (sensitive) - + id = "i-02ae66f368e8518a9" - + list_field = [ + + ami = (sensitive) + + id = "i-02ae66f368e8518a9" + + list_field = [ + "hello", + (sensitive), + "!", ] - + map_key = { + + map_key = { + "breakfast" = 800 + "dinner" = (sensitive) } - + map_whole = (sensitive) + + map_whole = (sensitive) + + nested_sensitive = (sensitive) + nested_block_list { # At least one attribute in this block is (or was) sensitive, @@ -4945,6 +5007,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "an_attr": cty.StringVal("secretval"), }), }), + "nested_sensitive": cty.ObjectVal(map[string]cty.Value{ + "an_attr": cty.StringVal("secret because nested_sensitive is sensitive"), + }), }), After: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), @@ -4974,6 +5039,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "an_attr": cty.StringVal("changed"), }), }), + "nested_sensitive": cty.ObjectVal(map[string]cty.Value{ + "an_attr": cty.StringVal("changed"), + }), }), BeforeValMarks: []cty.PathValueMarks{ { @@ -5019,6 +5087,12 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "some_number": {Type: cty.Number, Optional: true}, "map_key": {Type: cty.Map(cty.Number), Optional: true}, "map_whole": {Type: cty.Map(cty.String), Optional: true}, + "nested_sensitive": {NestedType: &configschema.Object{ + Attributes: map[string]*configschema.Attribute{ + "an_attr": {Type: cty.String, Optional: true}, + }, + Nesting: configschema.NestingSingle, + }, Sensitive: true}, }, BlockTypes: map[string]*configschema.NestedBlock{ "nested_block": { @@ -5043,15 +5117,15 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { ~ resource "test_instance" "example" { # Warning: this attribute value will no longer be marked as sensitive # after applying this change. - ~ ami = (sensitive) - id = "i-02ae66f368e8518a9" - ~ list_field = [ + ~ ami = (sensitive) + id = "i-02ae66f368e8518a9" + ~ list_field = [ # (1 unchanged element hidden) "friends", - (sensitive), + ".", ] - ~ map_key = { + ~ map_key = { # Warning: this attribute value will no longer be marked as sensitive # after applying this change. ~ "dinner" = (sensitive) @@ -5059,13 +5133,14 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { } # Warning: this attribute value will no longer be marked as sensitive # after applying this change. - ~ map_whole = (sensitive) + ~ map_whole = (sensitive) + ~ nested_sensitive = (sensitive) # Warning: this attribute value will no longer be marked as sensitive # after applying this change. - ~ some_number = (sensitive) + ~ some_number = (sensitive) # Warning: this attribute value will no longer be marked as sensitive # after applying this change. - ~ special = (sensitive) + ~ special = (sensitive) # Warning: this block will no longer be marked as sensitive # after applying this change. @@ -5103,6 +5178,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "nested_block_single": cty.ObjectVal(map[string]cty.Value{ "an_attr": cty.StringVal("original"), }), + "nested_sensitive": cty.ObjectVal(map[string]cty.Value{ + "an_attr": cty.StringVal("secret because nested_sensitive is sensitive"), + }), }), After: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), @@ -5121,6 +5199,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "nested_block_single": cty.ObjectVal(map[string]cty.Value{ "an_attr": cty.StringVal("changed"), }), + "nested_sensitive": cty.ObjectVal(map[string]cty.Value{ + "an_attr": cty.StringVal("changed"), + }), }), AfterValMarks: []cty.PathValueMarks{ { @@ -5151,6 +5232,12 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "list_field": {Type: cty.List(cty.String), Optional: true}, "map_key": {Type: cty.Map(cty.Number), Optional: true}, "map_whole": {Type: cty.Map(cty.String), Optional: true}, + "nested_sensitive": {NestedType: &configschema.Object{ + Attributes: map[string]*configschema.Attribute{ + "an_attr": {Type: cty.String, Optional: true}, + }, + Nesting: configschema.NestingSingle, + }, Sensitive: true}, }, BlockTypes: map[string]*configschema.NestedBlock{ "nested_block_single": { @@ -5165,13 +5252,13 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { }, ExpectedOutput: ` # test_instance.example will be updated in-place ~ resource "test_instance" "example" { - id = "i-02ae66f368e8518a9" - ~ list_field = [ + id = "i-02ae66f368e8518a9" + ~ list_field = [ - "hello", + (sensitive), "friends", ] - ~ map_key = { + ~ map_key = { ~ "breakfast" = 800 -> 700 # Warning: this attribute value will be marked as sensitive and will not # display in UI output after applying this change. @@ -5179,7 +5266,8 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { } # Warning: this attribute value will be marked as sensitive and will not # display in UI output after applying this change. - ~ map_whole = (sensitive) + ~ map_whole = (sensitive) + ~ nested_sensitive = (sensitive) # Warning: this block will be marked as sensitive and will not # display in UI output after applying this change. @@ -5520,6 +5608,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "another": cty.StringVal("not secret"), }), }), + "nested_sensitive": cty.ObjectVal(map[string]cty.Value{ + "an_attr": cty.StringVal("secret because nested_sensitive is sensitive"), + }), }), After: cty.NullVal(cty.EmptyObject), BeforeValMarks: []cty.PathValueMarks{ @@ -5556,6 +5647,12 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "list_field": {Type: cty.List(cty.String), Optional: true}, "map_key": {Type: cty.Map(cty.Number), Optional: true}, "map_whole": {Type: cty.Map(cty.String), Optional: true}, + "nested_sensitive": {NestedType: &configschema.Object{ + Attributes: map[string]*configschema.Attribute{ + "an_attr": {Type: cty.String, Optional: true}, + }, + Nesting: configschema.NestingSingle, + }, Sensitive: true}, }, BlockTypes: map[string]*configschema.NestedBlock{ "nested_block_set": { @@ -5571,17 +5668,18 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { }, ExpectedOutput: ` # test_instance.example will be destroyed - resource "test_instance" "example" { - - ami = (sensitive) -> null - - id = "i-02ae66f368e8518a9" -> null - - list_field = [ + - ami = (sensitive) -> null + - id = "i-02ae66f368e8518a9" -> null + - list_field = [ - "hello", - (sensitive), ] -> null - - map_key = { + - map_key = { - "breakfast" = 800 - "dinner" = (sensitive) } -> null - - map_whole = (sensitive) -> null + - map_whole = (sensitive) -> null + - nested_sensitive = (sensitive) -> null - nested_block_set { # At least one attribute in this block is (or was) sensitive, @@ -5601,6 +5699,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "an_attr": cty.StringVal("secret"), }), }), + "nested_sensitive": cty.ObjectVal(map[string]cty.Value{ + "an_attr": cty.StringVal("secret because nested_sensitive is sensitive"), + }), }), After: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), @@ -5610,6 +5711,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "an_attr": cty.StringVal("changed"), }), }), + "nested_sensitive": cty.ObjectVal(map[string]cty.Value{ + "an_attr": cty.StringVal("changed"), + }), }), BeforeValMarks: []cty.PathValueMarks{ { @@ -5635,6 +5739,12 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { Attributes: map[string]*configschema.Attribute{ "id": {Type: cty.String, Optional: true, Computed: true}, "ami": {Type: cty.String, Optional: true}, + "nested_sensitive": {NestedType: &configschema.Object{ + Attributes: map[string]*configschema.Attribute{ + "an_attr": {Type: cty.String, Optional: true}, + }, + Nesting: configschema.NestingSingle, + }, Sensitive: true}, }, BlockTypes: map[string]*configschema.NestedBlock{ "nested_block_set": { @@ -5650,11 +5760,13 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { RequiredReplace: cty.NewPathSet( cty.GetAttrPath("ami"), cty.GetAttrPath("nested_block_set"), + cty.GetAttrPath("nested_sensitive"), ), ExpectedOutput: ` # test_instance.example must be replaced -/+ resource "test_instance" "example" { - ~ ami = (sensitive) # forces replacement - id = "i-02ae66f368e8518a9" + ~ ami = (sensitive) # forces replacement + id = "i-02ae66f368e8518a9" + ~ nested_sensitive = (sensitive) # forces replacement ~ nested_block_set { # forces replacement # At least one attribute in this block is (or was) sensitive,