From e831182c8d9abd847b2d12d1f413d0fd831d8a7e Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 11 Feb 2019 14:18:58 -0800 Subject: [PATCH] plans/objchange: Hide sensitive attribute values in error messages Since these error messages get printed in Terraform's output and we encourage users to share them as part of bug reports, we should avoid including sensitive information in them to reduce the risk of accidental exposure. --- configs/configschema/implied_type.go | 21 ++++++++++++ plans/objchange/compatible.go | 12 +++++-- plans/objchange/compatible_test.go | 26 +++++++++++++++ plans/objchange/plan_valid.go | 12 +++++-- plans/objchange/plan_valid_test.go | 49 ++++++++++++++++++++++++++++ 5 files changed, 116 insertions(+), 4 deletions(-) diff --git a/configs/configschema/implied_type.go b/configs/configschema/implied_type.go index 67324ebce..c0ee8419d 100644 --- a/configs/configschema/implied_type.go +++ b/configs/configschema/implied_type.go @@ -19,3 +19,24 @@ func (b *Block) ImpliedType() cty.Type { return hcldec.ImpliedType(b.DecoderSpec()) } + +// ContainsSensitive returns true if any of the attributes of the receiving +// block or any of its descendent blocks are marked as sensitive. +// +// Blocks themselves cannot be sensitive as a whole -- sensitivity is a +// per-attribute idea -- but sometimes we want to include a whole object +// decoded from a block in some UI output, and that is safe to do only if +// none of the contained attributes are sensitive. +func (b *Block) ContainsSensitive() bool { + for _, attrS := range b.Attributes { + if attrS.Sensitive { + return true + } + } + for _, blockS := range b.BlockTypes { + if blockS.ContainsSensitive() { + return true + } + } + return false +} diff --git a/plans/objchange/compatible.go b/plans/objchange/compatible.go index 0a784f97b..f19e72d77 100644 --- a/plans/objchange/compatible.go +++ b/plans/objchange/compatible.go @@ -41,13 +41,21 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu return errs } - for name := range schema.Attributes { + for name, attrS := range schema.Attributes { plannedV := planned.GetAttr(name) actualV := actual.GetAttr(name) path := append(path, cty.GetAttrStep{Name: name}) moreErrs := assertValueCompatible(plannedV, actualV, path) - errs = append(errs, moreErrs...) + if attrS.Sensitive { + if len(moreErrs) > 0 { + // Use a vague placeholder message instead, to avoid disclosing + // sensitive information. + errs = append(errs, path.NewErrorf("inconsistent values for sensitive attribute")) + } + } else { + errs = append(errs, moreErrs...) + } } for name, blockS := range schema.BlockTypes { plannedV := planned.GetAttr(name) diff --git a/plans/objchange/compatible_test.go b/plans/objchange/compatible_test.go index a90cbdfad..01050912c 100644 --- a/plans/objchange/compatible_test.go +++ b/plans/objchange/compatible_test.go @@ -95,6 +95,32 @@ func TestAssertObjectCompatible(t *testing.T) { `.name: was cty.StringVal("wotsit"), but now cty.StringVal("thingy")`, }, }, + { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + "name": { + Type: cty.String, + Required: true, + Sensitive: true, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "name": cty.StringVal("wotsit"), + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "name": cty.StringVal("thingy"), + }), + []string{ + `.name: inconsistent values for sensitive attribute`, + }, + }, { &configschema.Block{ Attributes: map[string]*configschema.Attribute{ diff --git a/plans/objchange/plan_valid.go b/plans/objchange/plan_valid.go index a21aad4e1..308c47f59 100644 --- a/plans/objchange/plan_valid.go +++ b/plans/objchange/plan_valid.go @@ -251,9 +251,17 @@ func assertPlannedValueValid(attrS *configschema.Attribute, priorV, configV, pla // If none of the above conditions match, the provider has made an invalid // change to this attribute. if priorV.IsNull() { - errs = append(errs, path.NewErrorf("planned value %#v does not match config value %#v", plannedV, configV)) + if attrS.Sensitive { + errs = append(errs, path.NewErrorf("sensitive planned value does not match config value")) + } else { + errs = append(errs, path.NewErrorf("planned value %#v does not match config value %#v", plannedV, configV)) + } return errs } - errs = append(errs, path.NewErrorf("planned value %#v does not match config value %#v nor prior value %#v", plannedV, configV, priorV)) + if attrS.Sensitive { + errs = append(errs, path.NewErrorf("sensitive planned value does not match config value nor prior value")) + } else { + errs = append(errs, path.NewErrorf("planned value %#v does not match config value %#v nor prior value %#v", plannedV, configV, priorV)) + } return errs } diff --git a/plans/objchange/plan_valid_test.go b/plans/objchange/plan_valid_test.go index 434a054e2..ff01881b5 100644 --- a/plans/objchange/plan_valid_test.go +++ b/plans/objchange/plan_valid_test.go @@ -167,6 +167,55 @@ func TestAssertPlanValid(t *testing.T) { `.b[0].c: planned value cty.StringVal("new c value") does not match config value cty.StringVal("c value")`, }, }, + "no computed, invalid change in plan sensitive": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "a": { + Type: cty.String, + Optional: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "b": { + Nesting: configschema.NestingList, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "c": { + Type: cty.String, + Optional: true, + Sensitive: true, + }, + }, + }, + }, + }, + }, + cty.NullVal(cty.Object(map[string]cty.Type{ + "a": cty.String, + "b": cty.List(cty.Object(map[string]cty.Type{ + "c": cty.String, + })), + })), + cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("a value"), + "b": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "c": cty.StringVal("c value"), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("a value"), + "b": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "c": cty.StringVal("new c value"), + }), + }), + }), + []string{ + `.b[0].c: sensitive planned value does not match config value`, + }, + }, "no computed, diff suppression in plan": { &configschema.Block{ Attributes: map[string]*configschema.Attribute{