Skip to content

Commit

Permalink
Avoid double-marking variables
Browse files Browse the repository at this point in the history
It is possible, say with multiple layers of
sensitive variables, to "double-mark" a variable.
Add a check to ensure this does not happen.
  • Loading branch information
pselle committed Dec 4, 2020
1 parent 6bacc7a commit 12b5d43
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 11 deletions.
33 changes: 25 additions & 8 deletions terraform/context_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5603,6 +5603,12 @@ func TestContext2Plan_variableSensitivityModule(t *testing.T) {
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
},
Variables: InputValues{
"another_var": &InputValue{
Value: cty.StringVal("boop"),
SourceType: ValueFromCaller,
},
},
})

plan, diags := ctx.Plan()
Expand All @@ -5628,21 +5634,32 @@ func TestContext2Plan_variableSensitivityModule(t *testing.T) {
switch i := ric.Addr.String(); i {
case "module.child.aws_instance.foo":
checkVals(t, objectVal(t, schema, map[string]cty.Value{
"foo": cty.StringVal("foo"),
"foo": cty.StringVal("foo"),
"value": cty.StringVal("boop"),
}), ric.After)
if len(res.ChangeSrc.BeforeValMarks) != 0 {
t.Errorf("unexpected BeforeValMarks: %#v", res.ChangeSrc.BeforeValMarks)
}
if len(res.ChangeSrc.AfterValMarks) != 1 {
t.Errorf("unexpected AfterValMarks: %#v", res.ChangeSrc.AfterValMarks)
if len(res.ChangeSrc.AfterValMarks) != 2 {
t.Errorf("expected AfterValMarks to contain two elements: %#v", res.ChangeSrc.AfterValMarks)
continue
}
pvm := res.ChangeSrc.AfterValMarks[0]
if got, want := pvm.Path, cty.GetAttrPath("foo"); !got.Equals(want) {
t.Errorf("unexpected path for mark\n got: %#v\nwant: %#v", got, want)
// validate that the after marks have "foo" and "value"
contains := func(pvmSlice []cty.PathValueMarks, stepName string) bool {
for _, pvm := range pvmSlice {
if pvm.Path.Equals(cty.GetAttrPath(stepName)) {
if pvm.Marks.Equal(cty.NewValueMarks("sensitive")) {
return true
}
}
}
return false
}
if got, want := pvm.Marks, cty.NewValueMarks("sensitive"); !got.Equal(want) {
t.Errorf("unexpected value for mark\n got: %#v\nwant: %#v", got, want)
if !contains(res.ChangeSrc.AfterValMarks, "foo") {
t.Error("unexpected AfterValMarks to contain \"foo\" with sensitive mark")
}
if !contains(res.ChangeSrc.AfterValMarks, "value") {
t.Error("unexpected AfterValMarks to contain \"value\" with sensitive mark")
}
default:
t.Fatal("unknown instance:", i)
Expand Down
3 changes: 2 additions & 1 deletion terraform/evaluate.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd
val = cty.UnknownVal(wantType)
}

if config.Sensitive {
// Mark if sensitive, and avoid double-marking if this has already been marked
if config.Sensitive && !val.HasMark("sensitive") {
val = val.Mark("sensitive")
}

Expand Down
23 changes: 22 additions & 1 deletion terraform/evaluate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,20 @@ func TestEvaluatorGetInputVariable(t *testing.T) {
Sensitive: true,
Default: cty.StringVal("foo"),
},
// Avoid double marking a value
"some_other_var": {
Name: "some_other_var",
Sensitive: true,
Default: cty.StringVal("bar"),
},
},
},
},
VariableValues: map[string]map[string]cty.Value{
"": {"some_var": cty.StringVal("bar")},
"": {
"some_var": cty.StringVal("bar"),
"some_other_var": cty.StringVal("boop").Mark("sensitive"),
},
},
VariableValuesLock: &sync.Mutex{},
}
Expand All @@ -124,6 +133,18 @@ func TestEvaluatorGetInputVariable(t *testing.T) {
if !got.RawEquals(want) {
t.Errorf("wrong result %#v; want %#v", got, want)
}

want = cty.StringVal("boop").Mark("sensitive")
got, diags = scope.Data.GetInputVariable(addrs.InputVariable{
Name: "some_other_var",
}, tfdiags.SourceRange{})

if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
}
if !got.RawEquals(want) {
t.Errorf("wrong result %#v; want %#v", got, want)
}
}

func TestEvaluatorGetResource(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ variable "foo" {
type = string
}

// "bar" is defined as sensitive by both the parent and the child
variable "bar" {
sensitive = true
}

resource "aws_instance" "foo" {
foo = var.foo
foo = var.foo
value = var.bar
}
5 changes: 5 additions & 0 deletions terraform/testdata/plan-variable-sensitivity-module/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ variable "sensitive_var" {
sensitive = true
}

variable "another_var" {
sensitive = true
}

module "child" {
source = "./child"
foo = var.sensitive_var
bar = var.another_var
}

0 comments on commit 12b5d43

Please sign in to comment.