From 190079b7f358a5e75e924541533bb7e6dbf0b506 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sat, 5 Jul 2025 10:36:05 +0200 Subject: [PATCH] feat: reporter helper to mask secrets, including multiline - the longest secret is masked first - multiline secrets are masked before single line secrets - O(multiline * log rows) to not degrade performances when there are no multiline secrets --- internal/pkg/report/mask.go | 146 ++++++++++++++++++ internal/pkg/report/mask_test.go | 257 +++++++++++++++++++++++++++++++ 2 files changed, 403 insertions(+) create mode 100644 internal/pkg/report/mask.go create mode 100644 internal/pkg/report/mask_test.go diff --git a/internal/pkg/report/mask.go b/internal/pkg/report/mask.go new file mode 100644 index 00000000..1e7bbeb0 --- /dev/null +++ b/internal/pkg/report/mask.go @@ -0,0 +1,146 @@ +// Copyright 2025 The Forgejo Authors. +// SPDX-License-Identifier: MIT + +package report + +import ( + "cmp" + "slices" + "strings" + + runnerv1 "code.forgejo.org/forgejo/actions-proto/runner/v1" +) + +type masker struct { + replacer *strings.Replacer + lines []string + multiLines [][]string +} + +func newMasker() *masker { + return &masker{ + lines: make([]string, 0, 10), + multiLines: make([][]string, 0, 10), + } +} + +func (o *masker) add(secret string) { + if len(secret) == 0 { + return + } + o.replacer = nil + lines := strings.Split(strings.ReplaceAll(secret, "\r\n", "\n"), "\n") + if len(lines) > 1 { + o.multiLines = append(o.multiLines, lines) + // make sure the longest secret are replaced first + slices.SortFunc(o.multiLines, func(a, b []string) int { + return cmp.Compare(len(b), len(a)) + }) + } else { + o.lines = append(o.lines, lines[0]) + // make sure the longest secret are replaced first + slices.SortFunc(o.lines, func(a, b string) int { + return cmp.Compare(len(b), len(a)) + }) + } +} + +func (o *masker) getReplacer() *strings.Replacer { + if o.replacer == nil { + oldnew := make([]string, 0, len(o.lines)*2) + for _, line := range o.lines { + oldnew = append(oldnew, line, "***") + } + o.replacer = strings.NewReplacer(oldnew...) + } + return o.replacer +} + +func (o *masker) replaceLines(rows []*runnerv1.LogRow) { + r := o.getReplacer() + for _, row := range rows { + row.Content = r.Replace(row.Content) + } +} + +func (o *masker) maybeReplaceMultiline(multiLine []string, rows []*runnerv1.LogRow) bool { + equal, needMore := o.equalMultiline(multiLine, rows) + if needMore { + return needMore + } + if equal { + o.replaceMultiline(multiLine, rows) + } + return false +} + +func (o *masker) trimEOL(s string) string { + return strings.TrimRightFunc(s, func(r rune) bool { return r == '\r' || r == '\n' }) +} + +func (o *masker) equalMultiline(multiLine []string, rows []*runnerv1.LogRow) (equal, needMore bool) { + if len(rows) < 2 { + needMore = true + return + } + + lastIndex := len(multiLine) - 1 + first := multiLine[0] + if !strings.HasSuffix(o.trimEOL(rows[0].Content), first) { + return // unreachable because the caller checks that already + } + for i, line := range multiLine[1:lastIndex] { + rowIndex := i + 1 + if rowIndex >= len(rows) { + needMore = true + return + } + if o.trimEOL(rows[rowIndex].Content) != line { + return + } + } + last := multiLine[lastIndex] + if lastIndex >= len(rows) { + needMore = true + return + } + if !strings.HasPrefix(o.trimEOL(rows[lastIndex].Content), last) { + return + } + equal = true + return +} + +func (o *masker) replaceMultiline(multiLine []string, rows []*runnerv1.LogRow) { + lastIndex := len(multiLine) - 1 + first := multiLine[0] + rows[0].Content = strings.TrimSuffix(rows[0].Content, first) + "***" + for _, row := range rows[1:lastIndex] { + row.Content = "***" + } + last := multiLine[lastIndex] + rows[lastIndex].Content = "***" + strings.TrimPrefix(rows[lastIndex].Content, last) +} + +func (o *masker) replaceMultilines(rows []*runnerv1.LogRow) bool { + for _, multiLine := range o.multiLines { + for i, row := range rows { + if strings.HasSuffix(o.trimEOL(row.Content), multiLine[0]) { + needMore := o.maybeReplaceMultiline(multiLine, rows[i:]) + if needMore { + return needMore + } + } + } + } + return false +} + +func (o *masker) replace(rows []*runnerv1.LogRow, noMore bool) bool { + needMore := o.replaceMultilines(rows) + if !noMore && needMore { + return needMore + } + o.replaceLines(rows) + return false +} diff --git a/internal/pkg/report/mask_test.go b/internal/pkg/report/mask_test.go new file mode 100644 index 00000000..f6f332e4 --- /dev/null +++ b/internal/pkg/report/mask_test.go @@ -0,0 +1,257 @@ +// Copyright 2025 The Forgejo Authors. +// SPDX-License-Identifier: MIT + +package report + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestReporterMask(t *testing.T) { + lineOne := "secretOne" + lineTwo := lineOne + "secretTwoIsLongerThanAndStartsWithIt" + multiLineMixedSeparators := "A\nB\r\nC\r\nD" + multiLineOne := lineOne + ` +TWO +THREE` + multiLineTwo := multiLineOne + ` +FOUR +FIVE +SIX` + for _, testCase := range []struct { + name string + secrets []string + in string + out string + noMore bool + needMore bool + }{ + { + // + // a multiline secret is masked + // + name: "MultilineIsMasked", + secrets: []string{ + multiLineOne, + }, + in: fmt.Sprintf("line before\n%[1]s\nline after", multiLineOne), + out: "line before\n***\n***\n***\nline after\n", + needMore: false, + }, + { + // + // in a multiline secret \r\n is equivalent to \n and does + // not change how it is masked + // + name: "MultilineWithMixedLineSeparatorsIsMasked", + secrets: []string{ + multiLineMixedSeparators, + }, + in: fmt.Sprintf("line before\n%[1]s\nline after", multiLineMixedSeparators), + out: "line before\n***\n***\n***\n***\nline after\n", + needMore: false, + }, + { + // + // the last line of a multline secret is not a match + // + // + name: "MultilineLastLineDoesNotMatch", + secrets: []string{ + multiLineOne, + }, + in: fmt.Sprintf("%s\nTWO\nsomethingelse", lineOne), + out: lineOne + "\nTWO\nsomethingelse\n", + needMore: false, + }, + { + // + // non-multine secrets are masked + // + name: "SecretsAreMasked", + secrets: []string{ + "", + lineOne, + lineTwo, + }, + in: fmt.Sprintf("line before\n%[1]s\n%[2]s\nline after", lineOne, lineTwo), + out: "line before\n***\n***\nline after\n", + needMore: false, + }, + { + // + // the first line of a multiline secret may be found + // at the end of a line and the last line may be followed + // by a suffix, e.g. + // + // >>>ONE + // TWO + // THREE<<< + // + // and is expected to be replaced with + // + // >>>*** + // *** + // ***<<< + // + name: "MultilineWithSuffixAndPrefix", + secrets: []string{ + multiLineOne, + }, + in: fmt.Sprintf(">>>%[1]s<<<", multiLineOne), + out: ">>>***\n***\n***<<<\n", + needMore: false, + }, + { + // + // multiline secrets are considered first + // since only the first line of the multiLineOne secret + // is found, it needs more input to decide and does not + // mask anything. + // + // non-multiline secrets are not considered at all if + // a multiline secret needs more input and the + // lineOne secret is not masked even though it is found + // + // the first lines is found but not the second + // + name: "NeedMoreLines", + secrets: []string{ + lineOne, + multiLineOne, + }, + in: lineOne, + out: lineOne + "\n", + needMore: true, + }, + { + // + // the lines up to but not including the last are found + // + // See NeedMoreLines + // + name: "NeedMoreLinesVariation1", + secrets: []string{ + multiLineOne, + }, + in: fmt.Sprintf("%s\nTWO", lineOne), + out: lineOne + "\nTWO\n", + needMore: true, + }, + { + // + // the lines up to the third out of six are found + // + // See NeedMoreLines + // + name: "NeedMoreLinesVariation2", + secrets: []string{ + multiLineTwo, + }, + in: multiLineOne, + out: multiLineOne + "\n", + needMore: true, + }, + { + // + // a multiline secret will be masked if it is found + // even when another multiline secret needs more input + // + // however non-multiline secrets will not be masked + // + name: "NeedMoreLinesAndMultilinePartialMasking", + secrets: []string{ + lineOne, + multiLineOne, + }, + in: fmt.Sprintf(`%[1]s %[2]s +>>>%[3]s<<< +%[1]s`, lineOne, lineTwo, multiLineOne), + out: "secretOne secretOnesecretTwoIsLongerThanAndStartsWithIt\n>>>***\n***\n***<<<\nsecretOne\n", + needMore: true, + }, + { + // + // - oneline overlaps with lineTwo + // - oneLine overlaps with multiLineOne and multiLineTwo + // - multiLineOne overlaps with multiLineTwo + // + // they are all masked because the longest secret is masked + // first + // + name: "OverlappingSecrets", + secrets: []string{ + lineOne, + lineTwo, + multiLineOne, + multiLineTwo, + }, + in: fmt.Sprintf(`[[[%[1]s]]] {{{%[2]s}}} +>>>%[3]s<<< +(((%[4]s)))`, lineOne, lineTwo, multiLineOne, multiLineTwo), + out: `[[[***]]] {{{***}}} +>>>*** +*** +***<<< +(((*** +*** +*** +*** +*** +***))) +`, + needMore: false, + }, + { + // + // A multiline secret needing more lines does not + // prevent single line secrets from being masked + // when there is no more lines / these are the last + // available lines and there is no sense in hoping + // more will be available later. + // + name: "NeedMoreButNoMore", + secrets: []string{ + lineTwo, + multiLineOne, + }, + in: fmt.Sprintf(`[[[%[1]s]]] {{{%[2]s\nTWO}}}`, lineTwo, lineOne), + out: "[[[***]]] {{{secretOne\\nTWO}}}\n", + noMore: true, + needMore: false, + }, + { + // + // A variation where the partial multiline secret also + // happens to contain a single line secret (TWO) which + // needs to be masked. + // + // See NeedMoreButNoMore + // + name: "NeedMoreButNoMoreAndOverlappingSecret", + secrets: []string{ + "TWO", + lineTwo, + multiLineOne, + }, + in: fmt.Sprintf(`[[[%[1]s]]] {{{%[2]s\nTWO}}}`, lineTwo, lineOne), + out: "[[[***]]] {{{secretOne\\n***}}}\n", + noMore: true, + needMore: false, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + m := newMasker() + for _, secret := range testCase.secrets { + m.add(secret) + } + rows := stringToRows(testCase.in) + needMore := m.replace(rows, testCase.noMore) + assert.Equal(t, testCase.needMore, needMore) + assert.Equal(t, testCase.out, rowsToString(rows)) + }) + } +}