diff --git a/packer/logs.go b/packer/logs.go index dde95e738..def9579e5 100644 --- a/packer/logs.go +++ b/packer/logs.go @@ -4,16 +4,17 @@ package packer import ( - "bytes" "io" + "sort" "strings" "sync" ) type secretFilter struct { - s map[string]struct{} - m sync.Mutex - w io.Writer + s map[string]struct{} + sorted []string // non-empty secrets, longest first; rebuilt on Set + m sync.Mutex + w io.Writer } func (l *secretFilter) Set(secrets ...string) { @@ -22,6 +23,7 @@ func (l *secretFilter) Set(secrets ...string) { for _, s := range secrets { l.s[s] = struct{}{} } + l.sorted = sortedSecrets(l.s) } func (l *secretFilter) SetOutput(output io.Writer) { @@ -31,23 +33,77 @@ func (l *secretFilter) SetOutput(output io.Writer) { } func (l *secretFilter) Write(p []byte) (n int, err error) { - for s := range l.s { - if s != "" { - p = bytes.Replace(p, []byte(s), []byte(""), -1) - } - } - return l.w.Write(p) + secrets, w := l.snapshot() + return w.Write([]byte(redact(string(p), secrets))) } -// FilterString will overwrite any senstitive variables in a string, returning +// FilterString will overwrite any sensitive variables in a string, returning // the filtered string. func (l *secretFilter) FilterString(message string) string { - for s := range l.s { + secrets, _ := l.snapshot() + return redact(message, secrets) +} + +// snapshot reads the longest-first secret list and the output writer together +// under the mutex, so a concurrent Set or SetOutput can't race with a redaction +// in progress. Set replaces the slice wholesale rather than mutating it, so the +// returned slice stays safe to range over after the mutex is released. +func (l *secretFilter) snapshot() ([]string, io.Writer) { + l.m.Lock() + defer l.m.Unlock() + return l.sorted, l.w +} + +// redact replaces every occurrence of a secret with "". It scans the +// original input once, so a "" marker it writes is never itself +// searched for secrets. Feeding each replacement's output into the next lets a +// secret like "sitive" match text a previous replacement introduced, turning +// "my_token" into ">"; scanning the original avoids that. +// secrets must be ordered longest first so the longest match wins where secrets +// overlap at a position. +func redact(s string, secrets []string) string { + if len(secrets) == 0 { + return s + } + var b strings.Builder + b.Grow(len(s)) + for i := 0; i < len(s); { + matched := false + for _, secret := range secrets { + if strings.HasPrefix(s[i:], secret) { + b.WriteString("") + i += len(secret) + matched = true + break + } + } + if !matched { + b.WriteByte(s[i]) + i++ + } + } + return b.String() +} + +// sortedSecrets returns the non-empty secrets ordered longest first, and +// lexicographically for equal lengths so the order is deterministic. Redacting +// the longest match first keeps a shorter secret that is a substring of a +// longer one (e.g. "ubuntu" and "ubuntu-22.04") from leaving the remainder of +// the longer secret in the output. +func sortedSecrets(set map[string]struct{}) []string { + secrets := make([]string, 0, len(set)) + for s := range set { if s != "" { - message = strings.Replace(message, s, "", -1) + secrets = append(secrets, s) } } - return message + sort.Slice(secrets, func(i, j int) bool { + if len(secrets[i]) != len(secrets[j]) { + return len(secrets[i]) > len(secrets[j]) + } + return secrets[i] < secrets[j] + }) + return secrets } var LogSecretFilter secretFilter diff --git a/packer/logs_test.go b/packer/logs_test.go new file mode 100644 index 000000000..bbd5b15c9 --- /dev/null +++ b/packer/logs_test.go @@ -0,0 +1,82 @@ +// Copyright IBM Corp. 2013, 2025 +// SPDX-License-Identifier: MPL-2.0 + +package packer + +import ( + "bytes" + "io" + "reflect" + "sync" + "testing" +) + +func newSecretFilter(secrets ...string) *secretFilter { + l := &secretFilter{s: map[string]struct{}{}} + l.Set(secrets...) + return l +} + +// Set must record secrets longest first (lexicographically for equal lengths) +// and drop empty values, so redaction is deterministic regardless of the map's +// iteration order. +func TestSecretFilterSetSortsLongestFirst(t *testing.T) { + l := newSecretFilter("ubuntu", "", "ubuntu-22.04", "bb", "aa") + want := []string{"ubuntu-22.04", "ubuntu", "aa", "bb"} + if !reflect.DeepEqual(l.sorted, want) { + t.Fatalf("sorted secrets: got %q, want %q", l.sorted, want) + } +} + +// When one secret is a substring of another, the longest match must be redacted +// so the tail of the longer secret ("-22.04") can't leak. +func TestSecretFilterFilterStringOverlapping(t *testing.T) { + l := newSecretFilter("ubuntu-22.04", "ubuntu") + const want = "connecting to now" + if got := l.FilterString("connecting to ubuntu-22.04 now"); got != want { + t.Fatalf("secret partially leaked: got %q, want %q", got, want) + } +} + +// Redaction runs against the original input, so a secret that happens to appear +// inside the "" marker ("sitive") must not match text the marker +// introduced and turn "my_token" into ">". +func TestSecretFilterFilterStringDoesNotRefilterMarker(t *testing.T) { + l := newSecretFilter("my_token", "sitive") + if got := l.FilterString("my_token"); got != "" { + t.Fatalf("marker was re-filtered: got %q, want %q", got, "") + } +} + +func TestSecretFilterWriteOverlapping(t *testing.T) { + var buf bytes.Buffer + l := newSecretFilter("ubuntu-22.04", "ubuntu") + l.SetOutput(&buf) + + if _, err := l.Write([]byte("connecting to ubuntu-22.04 now")); err != nil { + t.Fatal(err) + } + const want = "connecting to now" + if got := buf.String(); got != want { + t.Fatalf("secret partially leaked: got %q, want %q", got, want) + } +} + +// Registering secrets while a redaction reads them must not race. Run under +// `go test -race`: reading the map without the mutex trips the detector. +func TestSecretFilterConcurrentSetAndRead(t *testing.T) { + l := newSecretFilter() + l.SetOutput(io.Discard) + + var wg sync.WaitGroup + for i := 0; i < 100; i++ { + wg.Add(3) + go func() { defer wg.Done(); l.Set("secret") }() + go func() { defer wg.Done(); l.FilterString("a secret in a message") }() + go func() { + defer wg.Done() + _, _ = l.Write([]byte("a secret in a message")) + }() + } + wg.Wait() +}