From 97952ced1f02f5619fea2bff304b6848b2114624 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Thu, 26 Mar 2026 12:04:48 +0530 Subject: [PATCH 1/2] BUG: Scrub multiline sensitive values from build output --- command/build_sensitive_multiline_test.go | 62 ++++++++++++++++++++++ hcl2template/parser.go | 3 +- packer/core.go | 2 +- packer/secrets.go | 34 ++++++++++++ packer/ui.go | 20 ++++--- packer/ui_test.go | 64 +++++++++++++++++++++++ 6 files changed, 174 insertions(+), 11 deletions(-) create mode 100644 command/build_sensitive_multiline_test.go create mode 100644 packer/secrets.go diff --git a/command/build_sensitive_multiline_test.go b/command/build_sensitive_multiline_test.go new file mode 100644 index 000000000..661d45c82 --- /dev/null +++ b/command/build_sensitive_multiline_test.go @@ -0,0 +1,62 @@ +// Copyright IBM Corp. 2013, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package command + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBuildScrubsSensitiveMultilineShellLocalOutput(t *testing.T) { + template := `variable "secret_multiline" { + type = string + sensitive = true + default = "line-one-secret\nline-two-secret\nline-three-secret" + } + + source "null" "example" { + communicator = "none" + } + + build { + sources = ["sources.null.example"] + + provisioner "shell-local" { + inline = [ + "printf 'BEGIN\n%s\nEND\n' '${var.secret_multiline}'" + ] + } + }` + + tmpDir := t.TempDir() + templatePath := filepath.Join(tmpDir, "multi-pwd.pkr.hcl") + if err := os.WriteFile(templatePath, []byte(template), 0o600); err != nil { + t.Fatalf("failed to write template: %v", err) + } + + c := &BuildCommand{ + Meta: TestMetaFile(t), + } + + if exitCode := c.Run([]string{templatePath}); exitCode != 0 { + out, stderr := GetStdoutAndErrFromTestMeta(t, c.Meta) + t.Fatalf("build failed with exit code %d\nstdout: %q\nstderr: %q", exitCode, out, stderr) + } + + out, stderr := GetStdoutAndErrFromTestMeta(t, c.Meta) + output := out + "\n" + stderr + secret := "line-one-secret\nline-two-secret\nline-three-secret" + + if strings.Contains(output, secret) { + t.Fatalf("multiline sensitive value leaked to build output: %q", output) + } + if strings.Contains(output, "line-one-secret") { + t.Fatalf("sensitive line leaked to build output: %q", output) + } + if !strings.Contains(output, "") { + t.Fatalf("expected scrubbed output, got: %q", output) + } +} diff --git a/hcl2template/parser.go b/hcl2template/parser.go index f92bb8db1..ca8b4e566 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/ext/dynblock" "github.com/hashicorp/hcl/v2/hclparse" - packersdk "github.com/hashicorp/packer-plugin-sdk/packer" "github.com/hashicorp/packer/internal/dag" "github.com/hashicorp/packer/packer" "github.com/zclconf/go-cty/cty" @@ -306,7 +305,7 @@ func filterVarsFromLogs(inputOrLocal Variables) { value := variable.Value() _ = cty.Walk(value, func(_ cty.Path, nested cty.Value) (bool, error) { if nested.IsWhollyKnown() && !nested.IsNull() && nested.Type().Equals(cty.String) { - packersdk.LogSecretFilter.Set(nested.AsString()) + packer.RegisterSecret(nested.AsString()) } return true, nil }) diff --git a/packer/core.go b/packer/core.go index d0f7501a6..ad3fc7356 100644 --- a/packer/core.go +++ b/packer/core.go @@ -171,7 +171,7 @@ func (core *Core) initialize() error { return err } for _, secret := range core.secrets { - packersdk.LogSecretFilter.Set(secret) + RegisterSecret(secret) } // Go through and interpolate all the build names. We should be able diff --git a/packer/secrets.go b/packer/secrets.go new file mode 100644 index 000000000..a4d59e24c --- /dev/null +++ b/packer/secrets.go @@ -0,0 +1,34 @@ +// Copyright IBM Corp. 2013, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package packer + +import ( + "strings" + + packersdk "github.com/hashicorp/packer-plugin-sdk/packer" +) + +func RegisterSecret(secret string) { + if secret == "" { + return + } + + secrets := map[string]struct{}{ + secret: {}, + } + + normalized := strings.ReplaceAll(secret, "\r\n", "\n") + secrets[normalized] = struct{}{} + + for _, line := range strings.Split(normalized, "\n") { + if line == "" { + continue + } + secrets[line] = struct{}{} + } + + for value := range secrets { + packersdk.LogSecretFilter.Set(value) + } +} diff --git a/packer/ui.go b/packer/ui.go index 70fd07e0f..f285ae59f 100644 --- a/packer/ui.go +++ b/packer/ui.go @@ -52,7 +52,7 @@ func (u *ColoredUi) Askf(query string, vals ...any) (string, error) { } func (u *ColoredUi) Say(message string) { - u.Ui.Say(u.colorize(message, u.Color, true)) + u.Ui.Say(u.colorize(scrubSecrets(message), u.Color, true)) } func (u *ColoredUi) Sayf(message string, vals ...any) { @@ -70,7 +70,7 @@ func (u *ColoredUi) Error(message string) { color = UiColorRed } - u.Ui.Error(u.colorize(message, color, true)) + u.Ui.Error(u.colorize(scrubSecrets(message), color, true)) } func (u *ColoredUi) Errorf(message string, vals ...any) { @@ -139,7 +139,7 @@ func (u *TargetedUI) Askf(query string, args ...any) (string, error) { } func (u *TargetedUI) Say(message string) { - u.Ui.Say(u.prefixLines(true, message)) + u.Ui.Say(u.prefixLines(true, scrubSecrets(message))) } func (u *TargetedUI) Sayf(message string, args ...any) { @@ -152,7 +152,7 @@ func (u *TargetedUI) Message(message string) { } func (u *TargetedUI) Error(message string) { - u.Ui.Error(u.prefixLines(true, message)) + u.Ui.Error(u.prefixLines(true, scrubSecrets(message))) } func (u *TargetedUI) Errorf(message string, args ...any) { @@ -235,8 +235,8 @@ func (u *MachineReadableUi) Machine(category string, args ...string) { // Prepare the args for i, v := range args { // Use packersdk.LogSecretFilter to scrub out sensitive variables - args[i] = packersdk.LogSecretFilter.FilterString(args[i]) - args[i] = strings.Replace(v, ",", "%!(PACKER_COMMA)", -1) + args[i] = scrubSecrets(v) + args[i] = strings.Replace(args[i], ",", "%!(PACKER_COMMA)", -1) args[i] = strings.Replace(args[i], "\r", "\\r", -1) args[i] = strings.Replace(args[i], "\n", "\\n", -1) } @@ -276,7 +276,7 @@ func (u *TimestampedUi) Askf(query string, args ...any) (string, error) { } func (u *TimestampedUi) Say(message string) { - u.Ui.Say(u.timestampLine(message)) + u.Ui.Say(u.timestampLine(scrubSecrets(message))) } func (u *TimestampedUi) Sayf(message string, args ...any) { @@ -289,7 +289,7 @@ func (u *TimestampedUi) Message(message string) { } func (u *TimestampedUi) Error(message string) { - u.Ui.Error(u.timestampLine(message)) + u.Ui.Error(u.timestampLine(scrubSecrets(message))) } func (u *TimestampedUi) Errorf(message string, args ...any) { @@ -307,3 +307,7 @@ func (u *TimestampedUi) TrackProgress(src string, currentSize, totalSize int64, func (u *TimestampedUi) timestampLine(string string) string { return fmt.Sprintf("%v: %v", time.Now().Format(time.RFC3339), string) } + +func scrubSecrets(message string) string { + return packersdk.LogSecretFilter.FilterString(message) +} diff --git a/packer/ui_test.go b/packer/ui_test.go index 7ed04009a..29a702bbd 100644 --- a/packer/ui_test.go +++ b/packer/ui_test.go @@ -5,6 +5,8 @@ package packer import ( "bytes" + "io" + "log" "strings" "testing" @@ -146,6 +148,27 @@ func TestTargetedUI(t *testing.T) { } } +func TestTargetedUI_ScrubsMultilineSecrets(t *testing.T) { + bufferUi := testUi() + targetedUi := &TargetedUI{ + Target: "null.example", + Ui: bufferUi, + } + + secret := "line-one-secret\nline-two-secret\nline-three-secret" + packersdk.LogSecretFilter.Set(secret) + + targetedUi.Say("BEGIN\n" + secret + "\nEND") + actual := readWriter(bufferUi) + + if strings.Contains(actual, secret) { + t.Fatalf("secret leaked in output: %q", actual) + } + if !strings.Contains(actual, "==> null.example: ") { + t.Fatalf("expected scrubbed output, got: %q", actual) + } +} + func TestColoredUi_ImplUi(t *testing.T) { var raw interface{} raw = &ColoredUi{} @@ -297,4 +320,45 @@ func TestMachineReadableUi(t *testing.T) { if data != expected { t.Fatalf("bad: %#v", data) } + + buf.Reset() + secret := "line-one-secret\nline-two-secret\nline-three-secret" + packersdk.LogSecretFilter.Set(secret) + ui.Machine("foo", secret) + data = buf.String() + if strings.Contains(data, "line-one-secret") { + t.Fatalf("secret leaked in machine-readable output: %q", data) + } + if !strings.Contains(data, "") { + t.Fatalf("expected scrubbed machine-readable output, got: %q", data) + } +} + +func TestLoggerScrubsCombinedMultilineSecrets(t *testing.T) { + secret := "line-one-secret\nline-two-secret\nline-three-secret" + RegisterSecret(secret) + + buf := new(bytes.Buffer) + packersdk.LogSecretFilter.SetOutput(buf) + + oldWriter := log.Writer() + oldFlags := log.Flags() + defer log.SetOutput(oldWriter) + defer log.SetFlags(oldFlags) + defer packersdk.LogSecretFilter.SetOutput(io.Discard) + + log.SetFlags(0) + log.SetOutput(&packersdk.LogSecretFilter) + log.Print("BEGIN\n" + secret + "\nEND") + + output := buf.String() + if strings.Contains(output, secret) { + t.Fatalf("combined multiline secret leaked to logger output: %q", output) + } + if strings.Contains(output, "line-one-secret") { + t.Fatalf("multiline secret line leaked to logger output: %q", output) + } + if !strings.Contains(output, "") { + t.Fatalf("expected scrubbed logger output, got: %q", output) + } } From 14c79779290ded218c10f60a34506c1ac73f9204 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Thu, 26 Mar 2026 12:25:55 +0530 Subject: [PATCH 2/2] Refactors sensitive multiline test to use OS-specific fixtures --- command/build_sensitive_multiline_test.go | 36 ++++++------------- .../multi-pwd.unix.pkr.hcl | 19 ++++++++++ .../multi-pwd.windows.pkr.hcl | 25 +++++++++++++ packer/ui.go | 6 ++-- 4 files changed, 57 insertions(+), 29 deletions(-) create mode 100644 command/test-fixtures/repro-sensitive-multiline/multi-pwd.unix.pkr.hcl create mode 100644 command/test-fixtures/repro-sensitive-multiline/multi-pwd.windows.pkr.hcl diff --git a/command/build_sensitive_multiline_test.go b/command/build_sensitive_multiline_test.go index 661d45c82..53e85be7e 100644 --- a/command/build_sensitive_multiline_test.go +++ b/command/build_sensitive_multiline_test.go @@ -4,38 +4,14 @@ package command import ( - "os" "path/filepath" + "runtime" "strings" "testing" ) func TestBuildScrubsSensitiveMultilineShellLocalOutput(t *testing.T) { - template := `variable "secret_multiline" { - type = string - sensitive = true - default = "line-one-secret\nline-two-secret\nline-three-secret" - } - - source "null" "example" { - communicator = "none" - } - - build { - sources = ["sources.null.example"] - - provisioner "shell-local" { - inline = [ - "printf 'BEGIN\n%s\nEND\n' '${var.secret_multiline}'" - ] - } - }` - - tmpDir := t.TempDir() - templatePath := filepath.Join(tmpDir, "multi-pwd.pkr.hcl") - if err := os.WriteFile(templatePath, []byte(template), 0o600); err != nil { - t.Fatalf("failed to write template: %v", err) - } + templatePath := filepath.Join(testFixture("repro-sensitive-multiline"), testBuildSensitiveMultilineShellLocalFixture(runtime.GOOS)) c := &BuildCommand{ Meta: TestMetaFile(t), @@ -60,3 +36,11 @@ func TestBuildScrubsSensitiveMultilineShellLocalOutput(t *testing.T) { t.Fatalf("expected scrubbed output, got: %q", output) } } + +func testBuildSensitiveMultilineShellLocalFixture(goos string) string { + if goos == "windows" { + return "multi-pwd.windows.pkr.hcl" + } + + return "multi-pwd.unix.pkr.hcl" +} diff --git a/command/test-fixtures/repro-sensitive-multiline/multi-pwd.unix.pkr.hcl b/command/test-fixtures/repro-sensitive-multiline/multi-pwd.unix.pkr.hcl new file mode 100644 index 000000000..87f68ac2f --- /dev/null +++ b/command/test-fixtures/repro-sensitive-multiline/multi-pwd.unix.pkr.hcl @@ -0,0 +1,19 @@ +variable "secret_multiline" { + type = string + sensitive = true + default = "line-one-secret\nline-two-secret\nline-three-secret" +} + +source "null" "example" { + communicator = "none" +} + +build { + sources = ["sources.null.example"] + + provisioner "shell-local" { + inline = [ + "printf 'BEGIN\n%s\nEND\n' '${var.secret_multiline}'" + ] + } +} \ No newline at end of file diff --git a/command/test-fixtures/repro-sensitive-multiline/multi-pwd.windows.pkr.hcl b/command/test-fixtures/repro-sensitive-multiline/multi-pwd.windows.pkr.hcl new file mode 100644 index 000000000..5085cba16 --- /dev/null +++ b/command/test-fixtures/repro-sensitive-multiline/multi-pwd.windows.pkr.hcl @@ -0,0 +1,25 @@ +variable "secret_multiline" { + type = string + sensitive = true + default = "line-one-secret\nline-two-secret\nline-three-secret" +} + +source "null" "example" { + communicator = "none" +} + +build { + sources = ["sources.null.example"] + + provisioner "shell-local" { + tempfile_extension = ".ps1" + environment_vars = ["SECRET_MULTILINE=${var.secret_multiline}"] + execute_command = ["powershell.exe", "{{.Vars}} {{.Script}}"] + env_var_format = "$env:%s=\"%s\"; " + inline = [ + "Write-Output 'BEGIN'", + "Write-Output $env:SECRET_MULTILINE", + "Write-Output 'END'" + ] + } +} \ No newline at end of file diff --git a/packer/ui.go b/packer/ui.go index f285ae59f..35c93e0e9 100644 --- a/packer/ui.go +++ b/packer/ui.go @@ -236,9 +236,9 @@ func (u *MachineReadableUi) Machine(category string, args ...string) { for i, v := range args { // Use packersdk.LogSecretFilter to scrub out sensitive variables args[i] = scrubSecrets(v) - args[i] = strings.Replace(args[i], ",", "%!(PACKER_COMMA)", -1) - args[i] = strings.Replace(args[i], "\r", "\\r", -1) - args[i] = strings.Replace(args[i], "\n", "\\n", -1) + args[i] = strings.ReplaceAll(args[i], ",", "%!(PACKER_COMMA)") + args[i] = strings.ReplaceAll(args[i], "\r", "\\r") + args[i] = strings.ReplaceAll(args[i], "\n", "\\n") } argsString := strings.Join(args, ",")