terraform: Hide maybe-sensitive provisioner output

If the provisioner configuration includes sensitive values, it's a
reasonable assumption that we should suppress its log output. Obvious
examples where this makes sense include echoing a secret to a file using
local-exec or remote-exec.

This commit adds tests for both logging output from provisioners with
non-sensitive configuration, and suppressing logs for provisioners with
sensitive values in configuration.

Note that we do not suppress logs if connection info contains sensitive
information, as provisioners should not be logging connection
information under any circumstances.
This commit is contained in:
Alisdair McDiarmid 2020-10-16 15:29:44 -04:00
parent 9c580335e3
commit 4f53234d8c
2 changed files with 40 additions and 1 deletions

View File

@ -4098,11 +4098,14 @@ func TestContext2Apply_Provisioner_compute(t *testing.T) {
if val != "computed_value" {
t.Fatalf("bad value for foo: %q", val)
}
req.UIOutput.Output(fmt.Sprintf("Executing: %q", val))
return
}
h := new(MockHook)
ctx := testContext2(t, &ContextOpts{
Config: m,
Hooks: []Hook{h},
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
},
@ -4136,6 +4139,14 @@ func TestContext2Apply_Provisioner_compute(t *testing.T) {
if !pr.ProvisionResourceCalled {
t.Fatalf("provisioner not invoked")
}
// Verify output was rendered
if !h.ProvisionOutputCalled {
t.Fatalf("ProvisionOutput hook not called")
}
if got, want := h.ProvisionOutputMessage, `Executing: "computed_value"`; got != want {
t.Errorf("expected output to be %q, but was %q", want, got)
}
}
func TestContext2Apply_provisionerCreateFail(t *testing.T) {
@ -12065,12 +12076,16 @@ func TestContext2Apply_provisionerSensitive(t *testing.T) {
if command.IsMarked() {
t.Fatalf("unexpectedly marked command argument: %#v", command.Marks())
}
req.UIOutput.Output(fmt.Sprintf("Executing: %q", command.AsString()))
return
}
p.ApplyResourceChangeFn = testApplyFn
p.PlanResourceChangeFn = testDiffFn
h := new(MockHook)
ctx := testContext2(t, &ContextOpts{
Config: m,
Hooks: []Hook{h},
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
},
@ -12106,4 +12121,15 @@ func TestContext2Apply_provisionerSensitive(t *testing.T) {
if !pr.ProvisionResourceCalled {
t.Fatalf("provisioner was not called on apply")
}
// Verify output was suppressed
if !h.ProvisionOutputCalled {
t.Fatalf("ProvisionOutput hook not called")
}
if got, doNotWant := h.ProvisionOutputMessage, "secret"; strings.Contains(got, doNotWant) {
t.Errorf("sensitive value %q included in output:\n%s", doNotWant, got)
}
if got, want := h.ProvisionOutputMessage, "output suppressed"; !strings.Contains(got, want) {
t.Errorf("expected hook to be called with %q, but was:\n%s", want, got)
}
}

View File

@ -687,9 +687,22 @@ func (n *EvalApplyProvisioners) apply(ctx EvalContext, provs []*configs.Provisio
// those are stripped out before sending to the provisioner. Unlike
// resources, we have no need to capture the marked paths and reapply
// later.
unmarkedConfig, _ := config.UnmarkDeep()
unmarkedConfig, configMarks := config.UnmarkDeep()
unmarkedConnInfo, _ := connInfo.UnmarkDeep()
// Marks on the config might result in leaking sensitive values through
// provisioner logging, so we conservatively suppress all output in
// this case. This should not apply to connection info values, which
// provisioners ought not to be logging anyway.
if len(configMarks) > 0 {
outputFn = func(msg string) {
ctx.Hook(func(h Hook) (HookAction, error) {
h.ProvisionOutput(absAddr, prov.Type, "(output suppressed due to sensitive value in config)")
return HookActionContinue, nil
})
}
}
output := CallbackUIOutput{OutputFn: outputFn}
resp := provisioner.ProvisionResource(provisioners.ProvisionResourceRequest{
Config: unmarkedConfig,