Merge pull request #3050 from buildkite/fix-multiline-secrets-with-crlf

Fix multiline secret redaction when output with \r\n

internal/replacer/replacer.go

@@ -4,6 +4,7 @@ package replacer
 import (
 	"fmt"
 	"io"
+	"strings"
 	"sync"
 )
 
@@ -122,24 +123,37 @@ func (r *Replacer) Write(b []byte) (int, error) {
 		// In the middle of matching?
 		for _, s := range r.partialMatches {
 			// Does the needle match on this byte?
-			if c != s.needle[s.matched] {
-				// No - drop this partial match.
+			switch s.needle[s.position] {
+			case c:
+				// It matched!
+				s.position++
+
+			case '\n':
+				// Special-case \n since PTY cooked-mode turns \n into \r\n.
+				// Allow repetitions of \r, as long as it is followed by \n
+				// (so don't increment s.position - \n should be matched later.
+				if c != '\r' {
+					continue
+				}
+
+			default:
+				// Did not match. Drop this partial match.
 				continue
 			}
 
-			// It matched!
+			// It's one more byte from the stream that matched.
 			s.matched++
 
 			// Have we fully matched this needle?
-			if s.matched < len(s.needle) {
+			if s.position < len(s.needle) {
 				// This state survives for another byte.
 				r.nextMatches = append(r.nextMatches, s)
 				continue
 			}
 
 			// Match complete; save range to redact.
 			r.completedMatches = append(r.completedMatches, subrange{
-				from: bufidx - len(s.needle) + 1,
+				from: bufidx - s.matched + 1,
 				to:   bufidx + 1,
 			})
 		}
@@ -156,8 +170,9 @@ func (r *Replacer) Write(b []byte) (int, error) {
 				continue
 			}
 			r.nextMatches = append(r.nextMatches, partialMatch{
-				needle:  s,
-				matched: 1,
+				needle:   s,
+				matched:  1,
+				position: 1,
 			})
 		}
 
@@ -343,21 +358,28 @@ func (r *Replacer) Add(needles ...string) {
 
 func (r *Replacer) unsafeAdd(needles []string) {
 	for _, s := range needles {
+		// Normalise all \r\n to \n in case a needle is supplied with \r\n line
+		// breaks but is output with \n for some reason. Note that the matcher
+		// matches \r+\n to \n, but one or more \r... without a \n afterwards
+		// in a needle should still be matched exactly.
+		s = strings.ReplaceAll(s, "\r\n", "\n")
 		if len(s) == 0 {
 			continue
 		}
-		if r.needlesByFirstByte[s[0]] == nil {
-			r.needlesByFirstByte[s[0]] = map[string]unit{s: {}}
+		firstByte := s[0]
+		if r.needlesByFirstByte[firstByte] == nil {
+			r.needlesByFirstByte[firstByte] = map[string]unit{s: {}}
 			continue
 		}
-		r.needlesByFirstByte[s[0]][s] = unit{}
+		r.needlesByFirstByte[firstByte][s] = unit{}
 	}
 }
 
 // partialMatch tracks how far through one of the needles we have matched.
 type partialMatch struct {
-	needle  string
-	matched int
+	needle   string
+	matched  int // number of bytes i the stream matched
+	position int // position within the needle matched up to
 }
 
 // subrange designates a contiguous range in a buffer (slice indexes: inclusive

internal/replacer/replacer_test.go

@@ -235,6 +235,51 @@ func TestReplacerMultiLine(t *testing.T) {
 	}
 }
 
+func TestReplacerMultiLineCrLf(t *testing.T) {
+	t.Parallel()
+
+	var buf strings.Builder
+
+	replacer := replacer.New(&buf, []string{"-----BEGIN OPENSSH PRIVATE KEY-----\nasdf\n-----END OPENSSH PRIVATE KEY-----\n"}, redact.Redact)
+
+	fmt.Fprint(replacer, "lalalala\r\n")
+	fmt.Fprint(replacer, "-----BEGIN OPENSSH PRIVATE KEY-----\r\n")
+	fmt.Fprint(replacer, "asdf\r\n")
+	fmt.Fprint(replacer, "-----END OPENSSH PRIVATE KEY-----\r\n")
+	fmt.Fprint(replacer, "lalalala\r\n")
+	replacer.Flush()
+
+	want := "lalalala\r\n[REDACTED]lalalala\r\n"
+
+	if diff := cmp.Diff(buf.String(), want); diff != "" {
+		t.Errorf("post-redaction diff (-got +want):\n%s", diff)
+	}
+}
+
+func TestReplacerMultiLineCrCrLf(t *testing.T) {
+	t.Parallel()
+
+	var buf strings.Builder
+
+	replacer := replacer.New(&buf, []string{"-----BEGIN OPENSSH PRIVATE KEY-----\nasdf\n-----END OPENSSH PRIVATE KEY-----\n"}, redact.Redact)
+
+	// Thanks to some combination of baked-mode PTY and other processing, log
+	// output linebreaks often look like \r\r\n, which is annoying both when
+	// redacting secrets and when opening them in a text editor.
+	fmt.Fprint(replacer, "lalalala\r\r\n")
+	fmt.Fprint(replacer, "-----BEGIN OPENSSH PRIVATE KEY-----\r\r\n")
+	fmt.Fprint(replacer, "asdf\r\r\n")
+	fmt.Fprint(replacer, "-----END OPENSSH PRIVATE KEY-----\r\r\n")
+	fmt.Fprint(replacer, "lalalala\r\r\n")
+	replacer.Flush()
+
+	want := "lalalala\r\r\n[REDACTED]lalalala\r\r\n"
+
+	if diff := cmp.Diff(buf.String(), want); diff != "" {
+		t.Errorf("post-redaction diff (-got +want):\n%s", diff)
+	}
+}
+
 func TestAddingNeedles(t *testing.T) {
 	t.Parallel()