From e27aacebf938f8e84f9d0d0ffcc0dfe826afb6bc Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Thu, 25 Mar 2021 11:41:49 -0400 Subject: [PATCH] command/jsonplan: Add sensitive value mapping data Similar to `after_unknown`, `before_sensitive` and `after_sensitive` are values with similar structure to `before` and `after` which encode the presence of sensitive values in a planned change. These should be used to obscure sensitive values from human-readable output. These values follow the same structure as the `before` and `after` values, replacing sensitive values with `true`, and non-sensitive values with `false`. Following the `after_unknown` precedent, we omit non-sensitive `false` values for object attributes/map values, to make serialization more compact. One difference from `after_unknown` is that a sensitive complex value (collection or structural type) is replaced with `true`. If the complex value itself is sensitive, all of its contents should be obscured. --- command/jsonplan/plan.go | 118 ++++++++++- command/jsonplan/plan_test.go | 197 ++++++++++++++++++ .../show-json/basic-create/output.json | 12 +- .../show-json/basic-delete/output.json | 8 +- .../show-json/basic-update/output.json | 4 +- .../show-json/module-depends-on/output.json | 4 +- .../testdata/show-json/modules/output.json | 16 +- .../multi-resource-update/output.json | 8 +- .../show-json/nested-modules/output.json | 4 +- .../provider-version-no-config/output.json | 12 +- .../show-json/provider-version/output.json | 12 +- .../show-json/sensitive-values/main.tf | 13 ++ .../show-json/sensitive-values/output.json | 114 ++++++++++ 13 files changed, 495 insertions(+), 27 deletions(-) create mode 100644 command/testdata/show-json/sensitive-values/main.tf create mode 100644 command/testdata/show-json/sensitive-values/output.json diff --git a/command/jsonplan/plan.go b/command/jsonplan/plan.go index ee12a408c..07d7f46b9 100644 --- a/command/jsonplan/plan.go +++ b/command/jsonplan/plan.go @@ -67,9 +67,23 @@ type change struct { // or "after" is unset (respectively). For ["no-op"], the before and after // values are identical. The "after" value will be incomplete if there are // values within it that won't be known until after apply. - Before json.RawMessage `json:"before,omitempty"` - After json.RawMessage `json:"after,omitempty"` + Before json.RawMessage `json:"before,omitempty"` + After json.RawMessage `json:"after,omitempty"` + + // AfterUnknown is an object value with similar structure to After, but + // with all unknown leaf values replaced with true, and all known leaf + // values omitted. This can be combined with After to reconstruct a full + // value after the action, including values which will only be known after + // apply. AfterUnknown json.RawMessage `json:"after_unknown,omitempty"` + + // BeforeSensitive and AfterSensitive are object values with similar + // structure to Before and After, but with all sensitive leaf values + // replaced with true, and all non-sensitive leaf values omitted. These + // objects should be combined with Before and After to prevent accidental + // display of sensitive values in user interfaces. + BeforeSensitive json.RawMessage `json:"before_sensitive,omitempty"` + AfterSensitive json.RawMessage `json:"after_sensitive,omitempty"` } type output struct { @@ -192,12 +206,18 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform } var before, after []byte + var beforeSensitive, afterSensitive []byte var afterUnknown cty.Value if changeV.Before != cty.NilVal { before, err = ctyjson.Marshal(changeV.Before, changeV.Before.Type()) if err != nil { return err } + bs := sensitiveAsBool(changeV.Before.MarkWithPaths(rc.BeforeValMarks)) + beforeSensitive, err = ctyjson.Marshal(bs, bs.Type()) + if err != nil { + return err + } } if changeV.After != cty.NilVal { if changeV.After.IsWhollyKnown() { @@ -218,6 +238,11 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform } afterUnknown = unknownAsBool(changeV.After) } + as := sensitiveAsBool(changeV.After.MarkWithPaths(rc.AfterValMarks)) + afterSensitive, err = ctyjson.Marshal(as, as.Type()) + if err != nil { + return err + } } a, err := ctyjson.Marshal(afterUnknown, afterUnknown.Type()) @@ -226,10 +251,12 @@ func (p *plan) marshalResourceChanges(changes *plans.Changes, schemas *terraform } r.Change = change{ - Actions: actionString(rc.Action.String()), - Before: json.RawMessage(before), - After: json.RawMessage(after), - AfterUnknown: a, + Actions: actionString(rc.Action.String()), + Before: json.RawMessage(before), + After: json.RawMessage(after), + AfterUnknown: a, + BeforeSensitive: json.RawMessage(beforeSensitive), + AfterSensitive: json.RawMessage(afterSensitive), } if rc.DeposedKey != states.NotDeposed { @@ -392,6 +419,9 @@ func omitUnknowns(val cty.Value) cty.Value { // tuple types and all mapping types are converted to object types, since we // assume the result of this is just going to be serialized as JSON (and thus // lose those distinctions) anyway. +// +// For map/object values, all known attribute values will be omitted instead of +// returning false, as this results in a more compact serialization. func unknownAsBool(val cty.Value) cty.Value { ty := val.Type() switch { @@ -439,7 +469,9 @@ func unknownAsBool(val cty.Value) cty.Value { for it.Next() { k, v := it.Element() vAsBool := unknownAsBool(v) - if !vAsBool.RawEquals(cty.False) { // all of the "false"s for known values for more compact serialization + // Omit all of the "false"s for known values for more compact + // serialization + if !vAsBool.RawEquals(cty.False) { vals[k.AsString()] = unknownAsBool(v) } } @@ -455,6 +487,78 @@ func unknownAsBool(val cty.Value) cty.Value { } } +// recursively iterate through a marked cty.Value, replacing sensitive values +// with cty.True and non-sensitive values with cty.False. +// +// The result also normalizes some types: all sequence types are turned into +// tuple types and all mapping types are converted to object types, since we +// assume the result of this is just going to be serialized as JSON (and thus +// lose those distinctions) anyway. +// +// For map/object values, all non-sensitive attribute values will be omitted +// instead of returning false, as this results in a more compact serialization. +func sensitiveAsBool(val cty.Value) cty.Value { + if val.HasMark("sensitive") { + return cty.True + } + + ty := val.Type() + switch { + case val.IsNull(), ty.IsPrimitiveType(), ty.Equals(cty.DynamicPseudoType): + return cty.False + case ty.IsListType() || ty.IsTupleType() || ty.IsSetType(): + length := val.LengthInt() + if length == 0 { + // If there are no elements then we can't have sensitive values + return cty.EmptyTupleVal + } + vals := make([]cty.Value, 0, length) + it := val.ElementIterator() + for it.Next() { + _, v := it.Element() + vals = append(vals, sensitiveAsBool(v)) + } + // The above transform may have changed the types of some of the + // elements, so we'll always use a tuple here in case we've now made + // different elements have different types. Our ultimate goal is to + // marshal to JSON anyway, and all of these sequence types are + // indistinguishable in JSON. + return cty.TupleVal(vals) + case ty.IsMapType() || ty.IsObjectType(): + var length int + switch { + case ty.IsMapType(): + length = val.LengthInt() + default: + length = len(val.Type().AttributeTypes()) + } + if length == 0 { + // If there are no elements then we can't have sensitive values + return cty.EmptyObjectVal + } + vals := make(map[string]cty.Value) + it := val.ElementIterator() + for it.Next() { + k, v := it.Element() + s := sensitiveAsBool(v) + // Omit all of the "false"s for non-sensitive values for more + // compact serialization + if !s.RawEquals(cty.False) { + vals[k.AsString()] = s + } + } + // The above transform may have changed the types of some of the + // elements, so we'll always use an object here in case we've now made + // different elements have different types. Our ultimate goal is to + // marshal to JSON anyway, and all of these mapping types are + // indistinguishable in JSON. + return cty.ObjectVal(vals) + default: + // Should never happen, since the above should cover all types + panic(fmt.Sprintf("sensitiveAsBool cannot handle %#v", val)) + } +} + func actionString(action string) []string { switch { case action == "NoOp": diff --git a/command/jsonplan/plan_test.go b/command/jsonplan/plan_test.go index 2a2e6b146..f6ee837fe 100644 --- a/command/jsonplan/plan_test.go +++ b/command/jsonplan/plan_test.go @@ -262,3 +262,200 @@ func TestUnknownAsBool(t *testing.T) { } } } + +func TestSensitiveAsBool(t *testing.T) { + sensitive := "sensitive" + tests := []struct { + Input cty.Value + Want cty.Value + }{ + { + cty.StringVal("hello"), + cty.False, + }, + { + cty.NullVal(cty.String), + cty.False, + }, + { + cty.StringVal("hello").Mark(sensitive), + cty.True, + }, + { + cty.NullVal(cty.String).Mark(sensitive), + cty.True, + }, + + { + cty.NullVal(cty.DynamicPseudoType).Mark(sensitive), + cty.True, + }, + { + cty.NullVal(cty.Object(map[string]cty.Type{"test": cty.String})), + cty.False, + }, + { + cty.NullVal(cty.Object(map[string]cty.Type{"test": cty.String})).Mark(sensitive), + cty.True, + }, + { + cty.DynamicVal, + cty.False, + }, + { + cty.DynamicVal.Mark(sensitive), + cty.True, + }, + + { + cty.ListValEmpty(cty.String), + cty.EmptyTupleVal, + }, + { + cty.ListValEmpty(cty.String).Mark(sensitive), + cty.True, + }, + { + cty.ListVal([]cty.Value{ + cty.StringVal("hello"), + cty.StringVal("friend").Mark(sensitive), + }), + cty.TupleVal([]cty.Value{ + cty.False, + cty.True, + }), + }, + { + cty.SetValEmpty(cty.String), + cty.EmptyTupleVal, + }, + { + cty.SetValEmpty(cty.String).Mark(sensitive), + cty.True, + }, + { + cty.SetVal([]cty.Value{cty.StringVal("hello")}), + cty.TupleVal([]cty.Value{cty.False}), + }, + { + cty.SetVal([]cty.Value{cty.StringVal("hello").Mark(sensitive)}), + cty.True, + }, + { + cty.EmptyTupleVal.Mark(sensitive), + cty.True, + }, + { + cty.TupleVal([]cty.Value{ + cty.StringVal("hello"), + cty.StringVal("friend").Mark(sensitive), + }), + cty.TupleVal([]cty.Value{ + cty.False, + cty.True, + }), + }, + { + cty.MapValEmpty(cty.String), + cty.EmptyObjectVal, + }, + { + cty.MapValEmpty(cty.String).Mark(sensitive), + cty.True, + }, + { + cty.MapVal(map[string]cty.Value{ + "greeting": cty.StringVal("hello"), + "animal": cty.StringVal("horse"), + }), + cty.EmptyObjectVal, + }, + { + cty.MapVal(map[string]cty.Value{ + "greeting": cty.StringVal("hello"), + "animal": cty.StringVal("horse").Mark(sensitive), + }), + cty.ObjectVal(map[string]cty.Value{ + "animal": cty.True, + }), + }, + { + cty.MapVal(map[string]cty.Value{ + "greeting": cty.StringVal("hello"), + "animal": cty.StringVal("horse").Mark(sensitive), + }).Mark(sensitive), + cty.True, + }, + { + cty.EmptyObjectVal, + cty.EmptyObjectVal, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "greeting": cty.StringVal("hello"), + "animal": cty.StringVal("horse"), + }), + cty.EmptyObjectVal, + }, + { + cty.ObjectVal(map[string]cty.Value{ + "greeting": cty.StringVal("hello"), + "animal": cty.StringVal("horse").Mark(sensitive), + }), + cty.ObjectVal(map[string]cty.Value{ + "animal": cty.True, + }), + }, + { + cty.ObjectVal(map[string]cty.Value{ + "greeting": cty.StringVal("hello"), + "animal": cty.StringVal("horse").Mark(sensitive), + }).Mark(sensitive), + cty.True, + }, + { + cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "a": cty.UnknownVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("known").Mark(sensitive), + }), + }), + cty.TupleVal([]cty.Value{ + cty.EmptyObjectVal, + cty.ObjectVal(map[string]cty.Value{ + "a": cty.True, + }), + }), + }, + { + cty.ListVal([]cty.Value{ + cty.MapValEmpty(cty.String), + cty.MapVal(map[string]cty.Value{ + "a": cty.StringVal("known").Mark(sensitive), + }), + cty.MapVal(map[string]cty.Value{ + "a": cty.UnknownVal(cty.String), + }), + }), + cty.TupleVal([]cty.Value{ + cty.EmptyObjectVal, + cty.ObjectVal(map[string]cty.Value{ + "a": cty.True, + }), + cty.EmptyObjectVal, + }), + }, + } + + for _, test := range tests { + got := sensitiveAsBool(test.Input) + if !reflect.DeepEqual(got, test.Want) { + t.Errorf( + "wrong result\ninput: %#v\ngot: %#v\nwant: %#v", + test.Input, got, test.Want, + ) + } + } +} diff --git a/command/testdata/show-json/basic-create/output.json b/command/testdata/show-json/basic-create/output.json index ff83de3fe..01a26d09b 100644 --- a/command/testdata/show-json/basic-create/output.json +++ b/command/testdata/show-json/basic-create/output.json @@ -83,7 +83,9 @@ }, "after": { "ami": "bar" - } + }, + "after_sensitive": {}, + "before_sensitive": false } }, { @@ -103,7 +105,9 @@ }, "after": { "ami": "bar" - } + }, + "after_sensitive": {}, + "before_sensitive": false } }, { @@ -123,7 +127,9 @@ }, "after": { "ami": "bar" - } + }, + "after_sensitive": {}, + "before_sensitive": false } } ], diff --git a/command/testdata/show-json/basic-delete/output.json b/command/testdata/show-json/basic-delete/output.json index f9580b401..f9efd426f 100644 --- a/command/testdata/show-json/basic-delete/output.json +++ b/command/testdata/show-json/basic-delete/output.json @@ -48,7 +48,9 @@ "ami": "bar", "id": "placeholder" }, - "after_unknown": {} + "after_unknown": {}, + "after_sensitive": {}, + "before_sensitive": {} } }, { @@ -66,7 +68,9 @@ "id": "placeholder" }, "after": null, - "after_unknown": {} + "after_unknown": {}, + "after_sensitive": false, + "before_sensitive": {} } } ], diff --git a/command/testdata/show-json/basic-update/output.json b/command/testdata/show-json/basic-update/output.json index 5486bece2..8a2f4de6f 100644 --- a/command/testdata/show-json/basic-update/output.json +++ b/command/testdata/show-json/basic-update/output.json @@ -48,7 +48,9 @@ "ami": "bar", "id": "placeholder" }, - "after_unknown": {} + "after_unknown": {}, + "after_sensitive": {}, + "before_sensitive": {} } } ], diff --git a/command/testdata/show-json/module-depends-on/output.json b/command/testdata/show-json/module-depends-on/output.json index 26e7e218b..5bfb89694 100644 --- a/command/testdata/show-json/module-depends-on/output.json +++ b/command/testdata/show-json/module-depends-on/output.json @@ -35,7 +35,9 @@ }, "after_unknown": { "id": true - } + }, + "after_sensitive": {}, + "before_sensitive": false } } ], diff --git a/command/testdata/show-json/modules/output.json b/command/testdata/show-json/modules/output.json index 898763aad..b78a9d1ab 100644 --- a/command/testdata/show-json/modules/output.json +++ b/command/testdata/show-json/modules/output.json @@ -99,7 +99,9 @@ }, "after_unknown": { "id": true - } + }, + "after_sensitive": {}, + "before_sensitive": false } }, { @@ -120,7 +122,9 @@ }, "after_unknown": { "id": true - } + }, + "after_sensitive": {}, + "before_sensitive": false } }, { @@ -141,7 +145,9 @@ }, "after_unknown": { "id": true - } + }, + "after_sensitive": {}, + "before_sensitive": false } }, { @@ -162,7 +168,9 @@ }, "after_unknown": { "id": true - } + }, + "after_sensitive": {}, + "before_sensitive": false } } ], diff --git a/command/testdata/show-json/multi-resource-update/output.json b/command/testdata/show-json/multi-resource-update/output.json index 6725c2546..a0418499f 100644 --- a/command/testdata/show-json/multi-resource-update/output.json +++ b/command/testdata/show-json/multi-resource-update/output.json @@ -63,7 +63,9 @@ "ami": "bar", "id": "placeholder" }, - "after_unknown": {} + "after_unknown": {}, + "after_sensitive": {}, + "before_sensitive": {} } }, { @@ -83,7 +85,9 @@ }, "after_unknown": { "id": true - } + }, + "after_sensitive": {}, + "before_sensitive": false } } ], diff --git a/command/testdata/show-json/nested-modules/output.json b/command/testdata/show-json/nested-modules/output.json index 369a58a4d..96e6b39e9 100644 --- a/command/testdata/show-json/nested-modules/output.json +++ b/command/testdata/show-json/nested-modules/output.json @@ -45,7 +45,9 @@ }, "after_unknown": { "id": true - } + }, + "after_sensitive": {}, + "before_sensitive": false } } ], diff --git a/command/testdata/show-json/provider-version-no-config/output.json b/command/testdata/show-json/provider-version-no-config/output.json index bef6f9b00..616376331 100644 --- a/command/testdata/show-json/provider-version-no-config/output.json +++ b/command/testdata/show-json/provider-version-no-config/output.json @@ -83,7 +83,9 @@ }, "after": { "ami": "bar" - } + }, + "after_sensitive": {}, + "before_sensitive": false } }, { @@ -103,7 +105,9 @@ }, "after": { "ami": "bar" - } + }, + "after_sensitive": {}, + "before_sensitive": false } }, { @@ -123,7 +127,9 @@ }, "after": { "ami": "bar" - } + }, + "after_sensitive": {}, + "before_sensitive": false } } ], diff --git a/command/testdata/show-json/provider-version/output.json b/command/testdata/show-json/provider-version/output.json index 33807107d..df1540e31 100644 --- a/command/testdata/show-json/provider-version/output.json +++ b/command/testdata/show-json/provider-version/output.json @@ -83,7 +83,9 @@ }, "after": { "ami": "bar" - } + }, + "after_sensitive": {}, + "before_sensitive": false } }, { @@ -103,7 +105,9 @@ }, "after": { "ami": "bar" - } + }, + "after_sensitive": {}, + "before_sensitive": false } }, { @@ -123,7 +127,9 @@ }, "after": { "ami": "bar" - } + }, + "after_sensitive": {}, + "before_sensitive": false } } ], diff --git a/command/testdata/show-json/sensitive-values/main.tf b/command/testdata/show-json/sensitive-values/main.tf new file mode 100644 index 000000000..3f8ba824c --- /dev/null +++ b/command/testdata/show-json/sensitive-values/main.tf @@ -0,0 +1,13 @@ +variable "test_var" { + default = "boop" + sensitive = true +} + +resource "test_instance" "test" { + ami = var.test_var +} + +output "test" { + value = test_instance.test.ami + sensitive = true +} diff --git a/command/testdata/show-json/sensitive-values/output.json b/command/testdata/show-json/sensitive-values/output.json new file mode 100644 index 000000000..7f67bc1fa --- /dev/null +++ b/command/testdata/show-json/sensitive-values/output.json @@ -0,0 +1,114 @@ +{ + "format_version": "0.1", + "variables": { + "test_var": { + "value": "boop" + } + }, + "planned_values": { + "outputs": { + "test": { + "sensitive": true, + "value": "boop" + } + }, + "root_module": { + "resources": [ + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "boop" + } + } + ] + } + }, + "resource_changes": [ + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after": { + "ami": "boop" + }, + "after_unknown": { + "id": true + }, + "after_sensitive": { + "ami": true + }, + "before_sensitive": false + } + } + ], + "output_changes": { + "test": { + "actions": [ + "create" + ], + "before": null, + "after": "boop", + "after_unknown": false + } + }, + "prior_state": { + "format_version": "0.1", + "values": { + "outputs": { + "test": { + "sensitive": true, + "value": "boop" + } + }, + "root_module": {} + } + }, + "configuration": { + "root_module": { + "outputs": { + "test": { + "expression": { + "references": [ + "test_instance.test" + ] + }, + "sensitive": true + } + }, + "resources": [ + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_config_key": "test", + "schema_version": 0, + "expressions": { + "ami": { + "references": [ + "var.test_var" + ] + } + } + } + ], + "variables": { + "test_var": { + "default": "boop" + } + } + } + } +}