Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: Loosen output value sensitivity requirement #28472

Merged
merged 1 commit into from
Apr 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 30 additions & 0 deletions terraform/context_plan2_test.go
Expand Up @@ -2,6 +2,7 @@ package terraform

import (
"errors"
"strings"
"testing"

"github.com/hashicorp/terraform/addrs"
Expand Down Expand Up @@ -485,3 +486,32 @@ provider "test" {
t.Fatal(diags.Err())
}
}

func TestContext2Plan_invalidSensitiveModuleOutput(t *testing.T) {
m := testModuleInline(t, map[string]string{
"child/main.tf": `
output "out" {
value = sensitive("xyz")
}`,
"main.tf": `
module "child" {
source = "./child"
}

output "root" {
value = module.child.out
}`,
})

ctx := testContext2(t, &ContextOpts{
Config: m,
})

_, diags := ctx.Plan()
if !diags.HasErrors() {
t.Fatal("succeeded; want errors")
}
if got, want := diags.Err().Error(), "Output refers to sensitive values"; !strings.Contains(got, want) {
t.Fatalf("wrong error:\ngot: %s\nwant: message containing %q", got, want)
}
}
20 changes: 6 additions & 14 deletions terraform/context_validate_test.go
Expand Up @@ -1379,7 +1379,7 @@ resource "aws_instance" "foo" {
}
}

func TestContext2Validate_invalidSensitiveModuleOutput(t *testing.T) {
func TestContext2Validate_sensitiveRootModuleOutput(t *testing.T) {
m := testModuleInline(t, map[string]string{
"child/main.tf": `
variable "foo" {
Expand All @@ -1395,27 +1395,19 @@ module "child" {
source = "./child"
}

resource "aws_instance" "foo" {
foo = module.child.out
output "root" {
value = module.child.out
sensitive = true
}`,
})

p := testProvider("aws")
ctx := testContext2(t, &ContextOpts{
Config: m,
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
},
})

diags := ctx.Validate()
if !diags.HasErrors() {
t.Fatal("succeeded; want errors")
}
// Should get this error:
// Output refers to sensitive values: Expressions used in outputs can only refer to sensitive values if the sensitive attribute is true.
if got, want := diags.Err().Error(), "Output refers to sensitive values"; !strings.Contains(got, want) {
t.Fatalf("wrong error:\ngot: %s\nwant: message containing %q", got, want)
if diags.HasErrors() {
t.Fatal(diags.Err())
}
}

Expand Down
2 changes: 1 addition & 1 deletion terraform/evaluate.go
Expand Up @@ -459,7 +459,7 @@ func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.Sourc
continue
}

instance[cfg.Name] = change.After
instance[cfg.Name] = change.After.MarkWithPaths(changeSrc.AfterValMarks)

if change.Sensitive && !change.After.HasMark("sensitive") {
instance[cfg.Name] = change.After.Mark("sensitive")
Expand Down
28 changes: 18 additions & 10 deletions terraform/node_output.go
Expand Up @@ -275,16 +275,24 @@ func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) (diags
// depends_on expressions here too
diags = diags.Append(validateDependsOn(ctx, n.Config.DependsOn))

// Ensure that non-sensitive outputs don't include sensitive values
// For root module outputs in particular, an output value must be
// statically declared as sensitive in order to dynamically return
// a sensitive result, to help avoid accidental exposure in the state
// of a sensitive value that the user doesn't want to include there.
_, marks := val.UnmarkDeep()
_, hasSensitive := marks["sensitive"]
if !n.Config.Sensitive && hasSensitive {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Output refers to sensitive values",
Detail: "Expressions used in outputs can only refer to sensitive values if the sensitive attribute is true.",
Subject: n.Config.DeclRange.Ptr(),
})
if n.Addr.Module.IsRoot() {
if !n.Config.Sensitive && hasSensitive {
apparentlymart marked this conversation as resolved.
Show resolved Hide resolved
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Output refers to sensitive values",
Detail: `To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output containing sensitive data be explicitly marked as sensitive, to confirm your intent.

If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
sensitive = true`,
Subject: n.Config.DeclRange.Ptr(),
})
}
}
}

Expand Down Expand Up @@ -454,7 +462,7 @@ func (n *NodeApplyableOutput) setValue(state *states.SyncState, changes *plans.C
sensitiveChange := sensitiveBefore || n.Config.Sensitive

// strip any marks here just to be sure we don't panic on the True comparison
val, _ = val.UnmarkDeep()
unmarkedVal, _ := val.UnmarkDeep()

action := plans.Update
switch {
Expand All @@ -468,7 +476,7 @@ func (n *NodeApplyableOutput) setValue(state *states.SyncState, changes *plans.C
action = plans.Create

case val.IsWhollyKnown() &&
val.Equals(before).True() &&
unmarkedVal.Equals(before).True() &&
n.Config.Sensitive == sensitiveBefore:
// Sensitivity must also match to be a NoOp.
// Theoretically marks may not match here, but sensitivity is the
Expand Down