fix: support multiline secret masking (#1001)

* command logging exposes multiline secrets more often than before
* duplicated add-mask command in reporter now handles this as well

Closes #998
Co-authored-by: silverwind <2021+silverwind@noreply.gitea.com>
Co-authored-by: silverwind <me@silverwind.io>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1001
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com>
Co-authored-by: Christopher Homberger <christopher.homberger@web.de>
Co-committed-by: Christopher Homberger <christopher.homberger@web.de>
This commit is contained in:
Christopher Homberger
2026-05-29 19:58:15 +00:00
committed by silverwind
parent abec931d98
commit c7c4bd600a
5 changed files with 102 additions and 15 deletions
+2 -2
View File
@@ -51,7 +51,7 @@ func (rc *RunContext) commandHandler(ctx context.Context) common.LineHandler {
logger.Infof("%s", line) logger.Infof("%s", line)
return false return false
} }
arg = unescapeCommandData(arg) arg = UnescapeCommandData(arg)
kvPairs = unescapeKvPairs(kvPairs) kvPairs = unescapeKvPairs(kvPairs)
switch command { switch command {
case "set-env": case "set-env":
@@ -151,7 +151,7 @@ func parseKeyValuePairs(kvPairs, separator string) map[string]string {
return rtn return rtn
} }
func unescapeCommandData(arg string) string { func UnescapeCommandData(arg string) string {
escapeMap := map[string]string{ escapeMap := map[string]string{
"%25": "%", "%25": "%",
"%0D": "\r", "%0D": "\r",
+29 -8
View File
@@ -10,6 +10,7 @@ import (
"fmt" "fmt"
"io" "io"
"os" "os"
"slices"
"strings" "strings"
"sync" "sync"
@@ -166,9 +167,29 @@ func withStepLogger(ctx context.Context, stepNumber int, stepID, stepName, stage
type entryProcessor func(entry *logrus.Entry) *logrus.Entry type entryProcessor func(entry *logrus.Entry) *logrus.Entry
func AppendSecretMasker(oldnew []string, v string) []string {
ret := oldnew
for l := range strings.SplitSeq(v, "\n") {
tm := strings.TrimSpace(l)
// formatted JSON secrets could otherwise mask {,[,],} everywhere
if len(tm) > 1 {
ret = append(ret, tm, "***")
}
}
return ret
}
// valueMasker applies secrets and ::add-mask:: patterns to every log entry, including // valueMasker applies secrets and ::add-mask:: patterns to every log entry, including
// raw_output (command/stream) lines; there is no bypass by field. // raw_output (command/stream) lines; there is no bypass by field.
func valueMasker(insecureSecrets bool, secrets map[string]string) entryProcessor { func valueMasker(insecureSecrets bool, secrets map[string]string) entryProcessor {
var oldnew []string
for _, v := range secrets {
oldnew = AppendSecretMasker(oldnew, v)
}
oldnew = slices.Clip(oldnew)
defReplacer := strings.NewReplacer(oldnew...)
return func(entry *logrus.Entry) *logrus.Entry { return func(entry *logrus.Entry) *logrus.Entry {
if insecureSecrets { if insecureSecrets {
return entry return entry
@@ -176,16 +197,16 @@ func valueMasker(insecureSecrets bool, secrets map[string]string) entryProcessor
masks := Masks(entry.Context) masks := Masks(entry.Context)
for _, v := range secrets { if len(*masks) == 0 {
if v != "" { entry.Message = defReplacer.Replace(entry.Message)
entry.Message = strings.ReplaceAll(entry.Message, v, "***") } else {
} cmasker := oldnew
}
for _, v := range *masks { for _, v := range *masks {
if v != "" { cmasker = AppendSecretMasker(cmasker, v)
entry.Message = strings.ReplaceAll(entry.Message, v, "***")
} }
entry.Message = strings.NewReplacer(cmasker...).Replace(entry.Message)
} }
return entry return entry
+52
View File
@@ -0,0 +1,52 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package runner
import (
"strings"
"testing"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
)
func TestValueMasker(t *testing.T) {
table := []struct {
name string
lines string
secrets map[string]string
masks []string
disallowed []string
}{
{
name: "Multiline Private Key",
lines: "cat << EOF > private.key\nPRIVATE_KEY_BEGIN\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\nPRIVATE_KEY_END\nEOF",
secrets: map[string]string{
"PRIVATE_KEY": "PRIVATE_KEY_BEGIN\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\nPRIVATE_KEY_END",
},
disallowed: []string{"KEY", "dsdfseffefsefes", "PRIVATE_KEY_END"},
},
{
name: "Multiline Private Key in masks",
lines: "cat << EOF > private.key\nPRIVATE_KEY_BEGIN\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\nPRIVATE_KEY_END\nEOF",
masks: []string{"PRIVATE_KEY_BEGIN\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\ndsdfseffefsefes\nPRIVATE_KEY_END"},
disallowed: []string{"KEY", "dsdfseffefsefes", "PRIVATE_KEY_END"},
},
}
for _, entry := range table {
t.Run(entry.name, func(t *testing.T) {
ctx := WithMasks(t.Context(), &entry.masks)
masker := valueMasker(false, entry.secrets)
for line := range strings.SplitSeq(entry.lines, "\n") {
lentry := masker(&logrus.Entry{
Context: ctx,
Message: line,
})
for _, line := range entry.disallowed {
assert.NotContains(t, lentry.Message, line)
}
}
})
}
}
+6 -5
View File
@@ -13,6 +13,7 @@ import (
"sync/atomic" "sync/atomic"
"time" "time"
"gitea.com/gitea/runner/act/runner"
"gitea.com/gitea/runner/internal/pkg/client" "gitea.com/gitea/runner/internal/pkg/client"
"gitea.com/gitea/runner/internal/pkg/config" "gitea.com/gitea/runner/internal/pkg/config"
"gitea.com/gitea/runner/internal/pkg/metrics" "gitea.com/gitea/runner/internal/pkg/metrics"
@@ -73,13 +74,13 @@ type Reporter struct {
func NewReporter(ctx context.Context, cancel context.CancelFunc, client client.Client, task *runnerv1.Task, cfg *config.Config) *Reporter { func NewReporter(ctx context.Context, cancel context.CancelFunc, client client.Client, task *runnerv1.Task, cfg *config.Config) *Reporter {
var oldnew []string var oldnew []string
if v := task.Context.Fields["token"].GetStringValue(); v != "" { if v := task.Context.Fields["token"].GetStringValue(); v != "" {
oldnew = append(oldnew, v, "***") oldnew = runner.AppendSecretMasker(oldnew, v)
} }
if v := task.Context.Fields["gitea_runtime_token"].GetStringValue(); v != "" { if v := task.Context.Fields["gitea_runtime_token"].GetStringValue(); v != "" {
oldnew = append(oldnew, v, "***") oldnew = runner.AppendSecretMasker(oldnew, v)
} }
for _, v := range task.Secrets { for _, v := range task.Secrets {
oldnew = append(oldnew, v, "***") oldnew = runner.AppendSecretMasker(oldnew, v)
} }
rv := &Reporter{ rv := &Reporter{
@@ -689,7 +690,7 @@ func (r *Reporter) parseLogRow(entry *log.Entry) *runnerv1.LogRow {
matches := cmdRegex.FindStringSubmatch(content) matches := cmdRegex.FindStringSubmatch(content)
if matches != nil { if matches != nil {
if output := r.handleCommand(content, matches[1], matches[3]); output != nil { if output := r.handleCommand(content, matches[1], runner.UnescapeCommandData(matches[3])); output != nil {
content = *output content = *output
} else { } else {
return nil return nil
@@ -705,6 +706,6 @@ func (r *Reporter) parseLogRow(entry *log.Entry) *runnerv1.LogRow {
} }
func (r *Reporter) addMask(msg string) { func (r *Reporter) addMask(msg string) {
r.oldnew = append(r.oldnew, msg, "***") r.oldnew = runner.AppendSecretMasker(r.oldnew, msg)
r.logReplacer = strings.NewReplacer(r.oldnew...) r.logReplacer = strings.NewReplacer(r.oldnew...)
} }
+13
View File
@@ -50,6 +50,19 @@ func TestReporter_parseLogRow(t *testing.T) {
"foo *** bar", "foo *** bar",
}, },
}, },
{
"Add-mask-multiline", false,
[]string{
"foo mysecret bar",
"::add-mask::LINE1%0ALINE2",
"foo LINE1 bar",
},
[]string{
"foo mysecret bar",
"<nil>",
"foo *** bar",
},
},
{ {
"Debug enabled", true, "Debug enabled", true,
[]string{ []string{