diff --git a/README.md b/README.md index 4c3ac657..f01e4a75 100644 --- a/README.md +++ b/README.md @@ -390,6 +390,10 @@ See **[docs/job-hooks.md](docs/job-hooks.md)** for the execution order, environm Set `log.job.dir` to a path and the runner writes a copy of every task's log there as `-task-.log`: the rows exactly as Gitea received them, with the same secrets masked and the job's result on the last line. Off by default, and what Gitea shows does not change. +#### Secret masking + +A job's secrets and its `::add-mask::` values are hidden from what the runner writes and uploads: the job log, the local copy above, job summaries, and the names of the containers it creates. A job output carrying one is skipped with a warning rather than sent masked, as GitHub does, so a downstream `needs..outputs.` reading it is empty. + `log.job.retention` (default `168h`) is how long a log is kept, expired ones being deleted as new tasks start, and `log.job.max_size` (default `1GB`) caps one log. Keep `retention` above `runner.timeout` so a long job cannot outlive its own log, and prefer local disk, the file is written while the job runs. Only the runner's own user can read it. ### Example Deployments diff --git a/act/runner/expression_test.go b/act/runner/expression_test.go index 6a6e6906..4120d7b5 100644 --- a/act/runner/expression_test.go +++ b/act/runner/expression_test.go @@ -356,3 +356,25 @@ on: } } } + +func TestJobNameMasksSecrets(t *testing.T) { + workflow, err := model.ReadWorkflow(strings.NewReader(` +jobs: + a: + name: deploy ${{ secrets.A }} + b: + name: deploy ${{ secrets.B }} +`)) + require.NoError(t, err) + + runner := &runnerImpl{config: &Config{Secrets: map[string]string{"A": "s3cr3t-a", "B": "s3cr3t-b"}}} + containerName := func(jobID string) string { + rc := runner.newRunContext(t.Context(), &model.Run{JobID: jobID, Workflow: workflow}, nil) + assert.NotContains(t, rc.Name, "s3cr3t") + return rc.jobContainerName() + } + + a, b := containerName("a"), containerName("b") + assert.NotContains(t, a, "s3cr3t") // it reaches the container name, which no log masker covers + assert.NotEqual(t, a, b) // masking the name must not collapse two jobs onto one container +} diff --git a/act/runner/job_executor.go b/act/runner/job_executor.go index 66ac5d70..adf37868 100644 --- a/act/runner/job_executor.go +++ b/act/runner/job_executor.go @@ -388,7 +388,7 @@ func setJobResult(ctx context.Context, info jobInfo, rc *RunContext, success boo if rc.caller != nil { // set reusable workflow job result - rc.caller.setReusedWorkflowJobResult(rc.JobName, jobResult) // For Gitea + rc.caller.setReusedWorkflowJobResult(rc.Run.JobID, jobResult) // For Gitea return } @@ -487,7 +487,8 @@ func tryUploadJobSummary(ctx context.Context, rc *RunContext) { if !ok || len(body) == 0 { continue } - uploadJobSummary(ctx, client, base+strconv.Itoa(i)+"/summary", runtimeToken, body) + // Gitea renders summaries on the run page, so mask before the upload. + uploadJobSummary(ctx, client, base+strconv.Itoa(i)+"/summary", runtimeToken, []byte(rc.maskSecrets(string(body)))) } } diff --git a/act/runner/job_executor_test.go b/act/runner/job_executor_test.go index ec3240a0..fa6119b1 100644 --- a/act/runner/job_executor_test.go +++ b/act/runner/job_executor_test.go @@ -1096,3 +1096,37 @@ func TestJobSetContinueOnError(t *testing.T) { assert.True(t, j.ContinueOnError) }) } + +func TestTryUploadJobSummaryMasksSecrets(t *testing.T) { + var got string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + assert.NoError(t, err) + got = string(body) + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + cm := &containerMock{} + cm.On("GetContainerArchive", mock.Anything, "/var/run/act/workflow/step-summary-0.md").Return( + io.NopCloser(bytes.NewReader(tarArchive(t, tarEntry{ + name: "step-summary-0.md", body: "deployed true with s3cr3t and runtime-added via pr0xypw", + }))), + nil, + ).Once() + + rc := newJobSummaryRC(map[string]string{ + "GITEA_ACTIONS_CAPABILITIES": "job-summary", + "ACTIONS_RUNTIME_URL": server.URL, + "ACTIONS_RUNTIME_TOKEN": fakeRuntimeToken(34), + "GITEA_RUN_ID": "12", + }, cm, 1) + rc.Config.Secrets = map[string]string{"TOK": "s3cr3t", "ACTIONS_STEP_DEBUG": "true"} + rc.Config.ExtraMasks = []string{"pr0xypw"} + rc.Masks = []string{"runtime-added"} + + tryUploadJobSummary(context.Background(), rc) + + assert.Equal(t, "deployed true with *** and *** via ***", got) + cm.AssertExpectations(t) +} diff --git a/act/runner/logger.go b/act/runner/logger.go index 0c633f07..2bc597c0 100644 --- a/act/runner/logger.go +++ b/act/runner/logger.go @@ -122,7 +122,7 @@ func WithJobLogger(ctx context.Context, jobID, jobName string, config *Config, m logger.SetFormatter(&maskedFormatter{ Formatter: logger.Formatter, - masker: valueMasker(config.InsecureSecrets, config.Secrets), + masker: valueMasker(config.InsecureSecrets, config.maskers()), }) rtn := logger.WithFields(logrus.Fields{ "job": jobName, @@ -234,6 +234,17 @@ func jsonStringEscapeNoHTML(v string) string { return string(encoded[1 : len(encoded)-1]) } +// AppendSecretMaskers skips the debug settings, as GitHub does: they arrive as secrets, but +// masking "true" would corrupt unrelated log lines and drop job outputs that say it. +func AppendSecretMaskers(oldnew []string, secrets map[string]string) []string { + for k, v := range secrets { + if k != "ACTIONS_STEP_DEBUG" && k != "ACTIONS_RUNNER_DEBUG" { + oldnew = AppendSecretMasker(oldnew, v) + } + } + return oldnew +} + func AppendSecretMasker(oldnew []string, v string) []string { ret := oldnew @@ -269,13 +280,9 @@ func AppendSecretMasker(oldnew []string, v string) []string { // valueMasker applies secrets and ::add-mask:: patterns to every log entry, including // raw_output (command/stream) lines; there is no bypass by field. -func valueMasker(insecureSecrets bool, secrets map[string]string) entryProcessor { - var oldnew []string - for _, v := range secrets { - oldnew = AppendSecretMasker(oldnew, v) - } +func valueMasker(insecureSecrets bool, oldnew []string) entryProcessor { oldnew = slices.Clip(oldnew) - defReplacer := strings.NewReplacer(oldnew...) + defReplacer := NewSecretReplacer(oldnew) // A ::add-mask:: only ever appends to the job's mask slice, so the replacer built for // it stays valid until the slice grows. Cache it, keyed by the slice itself and its @@ -311,7 +318,7 @@ func valueMasker(insecureSecrets bool, secrets map[string]string) entryProcessor pairs = AppendSecretMasker(pairs, v) } masked = len(*masks) - replacer = strings.NewReplacer(pairs...) + replacer = NewSecretReplacer(pairs) } cmasker := replacer mu.Unlock() @@ -419,3 +426,39 @@ func checkIfTerminal(w io.Writer) bool { return false } } + +// maskSecrets hides this job's secrets in a value that reaches somewhere the log maskers cannot, +// such as a container name or a job summary. Masks added at runtime count, so a summary written +// after ::add-mask:: is covered too. +func (rc *RunContext) maskSecrets(value string) string { + oldnew := rc.Config.maskers() + for _, mask := range rc.Masks { + oldnew = AppendSecretMasker(oldnew, mask) + } + return NewSecretReplacer(oldnew).Replace(value) +} + +// maskers is every value this job's configuration says to hide, whatever the sink. +func (c *Config) maskers() []string { + oldnew := AppendSecretMaskers(nil, c.Secrets) + for _, mask := range c.ExtraMasks { + oldnew = AppendSecretMasker(oldnew, mask) + } + return oldnew +} + +// NewSecretReplacer masks the longest secret first. Replacer matches in argument order, so a +// secret that prefixes another would otherwise mask only that prefix and print the rest. +func NewSecretReplacer(oldnew []string) *strings.Replacer { + pairs := make([][2]string, 0, len(oldnew)/2) + for i := 0; i+1 < len(oldnew); i += 2 { + pairs = append(pairs, [2]string{oldnew[i], oldnew[i+1]}) + } + slices.SortFunc(pairs, func(a, b [2]string) int { return len(b[0]) - len(a[0]) }) + + sorted := make([]string, 0, len(pairs)*2) + for _, pair := range pairs { + sorted = append(sorted, pair[0], pair[1]) + } + return strings.NewReplacer(sorted...) +} diff --git a/act/runner/logger_test.go b/act/runner/logger_test.go index 23f082a8..14d05cff 100644 --- a/act/runner/logger_test.go +++ b/act/runner/logger_test.go @@ -47,7 +47,7 @@ func TestValueMasker(t *testing.T) { for _, entry := range table { t.Run(entry.name, func(t *testing.T) { ctx := WithMasks(t.Context(), &entry.masks) - masker := valueMasker(false, entry.secrets) + masker := valueMasker(false, AppendSecretMaskers(nil, entry.secrets)) for line := range strings.SplitSeq(entry.lines, "\n") { lentry := masker(&logrus.Entry{ Context: ctx, @@ -65,7 +65,7 @@ func TestValueMasker(t *testing.T) { // URL — must be masked as well: masking only the verbatim value leaks it. func TestValueMaskerEncodedSecrets(t *testing.T) { secret := `p@ss w"rd/1` - masker := valueMasker(false, map[string]string{"TOKEN": secret}) + masker := valueMasker(false, AppendSecretMaskers(nil, map[string]string{"TOKEN": secret})) for _, tc := range []struct { name string @@ -94,7 +94,7 @@ func TestValueMaskerEncodedSecrets(t *testing.T) { // form, so a JS-serialized JSON body does not leak it. func TestValueMaskerJSONEscapesBothWays(t *testing.T) { secret := `a"&c` - masker := valueMasker(false, map[string]string{"TOKEN": secret}) + masker := valueMasker(false, AppendSecretMaskers(nil, map[string]string{"TOKEN": secret})) for _, tc := range []struct { name string @@ -112,10 +112,20 @@ func TestValueMaskerJSONEscapesBothWays(t *testing.T) { } } +// With debug logging on, the job logger writes to stdout, which no reporter masks, so it has +// to hide the values that are not job secrets too. +func TestValueMaskerHidesExtraMasks(t *testing.T) { + masker := valueMasker(false, (&Config{ExtraMasks: []string{"pr0xypw"}}).maskers()) + + entry := masker(&logrus.Entry{Context: t.Context(), Message: "proxy is http://user:pr0xypw@proxy:3128"}) + + assert.Equal(t, "proxy is http://user:***@proxy:3128", entry.Message) +} + // ::add-mask:: values go through the same masker, so they get the same treatment. func TestValueMaskerEncodedMasks(t *testing.T) { masks := []string{"s3cr3t value"} - masker := valueMasker(false, nil) + masker := valueMasker(false, AppendSecretMaskers(nil, nil)) entry := masker(&logrus.Entry{ Context: WithMasks(t.Context(), &masks), @@ -131,7 +141,7 @@ func TestValueMaskerEncodedMasks(t *testing.T) { // the token to anyone who can decode the log. func TestValueMaskerBase64Alignments(t *testing.T) { secret := "s3cr3t-token-value" - masker := valueMasker(false, map[string]string{"TOKEN": secret}) + masker := valueMasker(false, AppendSecretMaskers(nil, map[string]string{"TOKEN": secret})) // One prefix per alignment: len%3 of 0, 1 and 2. for _, prefix := range []string{"x-access-token:", "user:", "ab:"} { @@ -155,7 +165,7 @@ func TestValueMaskerBase64Alignments(t *testing.T) { // The masker caches its replacer, so it has to notice both a mask appended to the same // slice and a composite action logging with a slice of its own. func TestValueMaskerCachedReplacerSeesNewMasks(t *testing.T) { - masker := valueMasker(false, map[string]string{"TOKEN": "secret-token"}) + masker := valueMasker(false, AppendSecretMaskers(nil, map[string]string{"TOKEN": "secret-token"})) mask := func(masks *[]string, message string) string { return masker(&logrus.Entry{Context: WithMasks(t.Context(), masks), Message: message}).Message } diff --git a/act/runner/reusable_workflow.go b/act/runner/reusable_workflow.go index 2654ba5c..6d46571d 100644 --- a/act/runner/reusable_workflow.go +++ b/act/runner/reusable_workflow.go @@ -236,7 +236,7 @@ func setReusedWorkflowCallerResult(rc *RunContext, runner *runnerImpl) common.Ex } if rc.caller != nil { - rc.caller.setReusedWorkflowJobResult(rc.JobName, reusedWorkflowJobResult) + rc.caller.setReusedWorkflowJobResult(rc.Run.JobID, reusedWorkflowJobResult) } else { // Serialize this shared Job.Result write against the other matrix combos // and setJobResult (same lockJob key). diff --git a/act/runner/run_context.go b/act/runner/run_context.go index 255d5d8f..15aed5ff 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -180,7 +180,8 @@ func (rc *RunContext) GetEnv() map[string]string { } func (rc *RunContext) jobContainerName() string { - nameParts := []string{rc.Config.ContainerNamePrefix, "WORKFLOW-" + rc.Run.Workflow.Name, "JOB-" + rc.Name} + // The job id, never evaluated, keeps two jobs apart when masking collapses their names. + nameParts := []string{rc.Config.ContainerNamePrefix, "WORKFLOW-" + rc.Run.Workflow.Name, "JOB-" + rc.Run.JobID, rc.Name} if rc.caller != nil { nameParts = append(nameParts, "CALLED-BY-"+rc.caller.runContext.JobName) } @@ -1026,7 +1027,7 @@ func (rc *RunContext) Executor() (common.Executor, error) { // unfinished. rc.caller is only set for reusable workflows. rc.result("failure") if rc.caller != nil { // For Gitea - rc.caller.setReusedWorkflowJobResult(rc.JobName, "failure") + rc.caller.setReusedWorkflowJobResult(rc.Run.JobID, "failure") } return err } @@ -1116,7 +1117,7 @@ func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) { if !runJob { if rc.caller != nil { // For Gitea - rc.caller.setReusedWorkflowJobResult(rc.JobName, "skipped") + rc.caller.setReusedWorkflowJobResult(rc.Run.JobID, "skipped") return false, nil } l.WithField("jobResult", "skipped").Debugf("Skipping job '%s' due to '%s'", job.Name, job.If.Value) diff --git a/act/runner/runner.go b/act/runner/runner.go index 08cf1e72..47567217 100644 --- a/act/runner/runner.go +++ b/act/runner/runner.go @@ -36,6 +36,7 @@ type Config struct { JSONLogger bool // use json or text logger Env map[string]string // env for containers Secrets map[string]string // list of secrets + ExtraMasks []string // values to hide that are not in Secrets, such as the proxy password Vars map[string]string // list of vars Token string // GitHub token InsecureSecrets bool // switch hiding output when printing to terminal @@ -238,7 +239,7 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor { maxJobNameLen = len(rc.String()) } if rc.caller != nil { // For Gitea - rc.caller.setReusedWorkflowJobResult(rc.JobName, "pending") + rc.caller.setReusedWorkflowJobResult(rc.Run.JobID, "pending") } stageExecutor = append(stageExecutor, func(ctx context.Context) error { jobName := fmt.Sprintf("%-*s", maxJobNameLen, rc.String()) @@ -324,7 +325,7 @@ func (runner *runnerImpl) newRunContext(ctx context.Context, run *model.Run, mat caller: runner.caller, } rc.ExprEval = rc.NewExpressionEvaluator(ctx) - rc.Name = rc.ExprEval.Interpolate(ctx, run.String()) + rc.Name = rc.maskSecrets(rc.ExprEval.Interpolate(ctx, run.String())) // Snapshot the job's pristine output expressions now, before any matrix combo runs and // rewrites the shared Job.Outputs (see interpolateOutputs). if job := run.Job(); job != nil { @@ -335,8 +336,9 @@ func (runner *runnerImpl) newRunContext(ctx context.Context, run *model.Run, mat } // For Gitea -func (c *caller) setReusedWorkflowJobResult(jobName, result string) { +// Keyed by job id, not name: only the values are read, and masking can collapse two names into one. +func (c *caller) setReusedWorkflowJobResult(jobID, result string) { c.updateResultLock.Lock() defer c.updateResultLock.Unlock() - c.reusedWorkflowJobResults[jobName] = result + c.reusedWorkflowJobResults[jobID] = result } diff --git a/internal/app/cmd/daemon.go b/internal/app/cmd/daemon.go index 1259d71a..787f4a63 100644 --- a/internal/app/cmd/daemon.go +++ b/internal/app/cmd/daemon.go @@ -24,6 +24,7 @@ import ( "gitea.com/gitea/runner/internal/pkg/labels" "gitea.com/gitea/runner/internal/pkg/lock" "gitea.com/gitea/runner/internal/pkg/metrics" + "gitea.com/gitea/runner/internal/pkg/report" "gitea.com/gitea/runner/internal/pkg/ver" "connectrpc.com/connect" @@ -283,7 +284,7 @@ func initLogging(cfg *config.Config) { FullTimestamp: true, CallerPrettyfier: callPrettyfier, } - log.SetFormatter(format) + log.SetFormatter(report.MaskingFormatter(format)) l := cfg.Log.Level if l == "" { diff --git a/internal/app/cmd/daemon_test.go b/internal/app/cmd/daemon_test.go index 215b6127..ca009ff9 100644 --- a/internal/app/cmd/daemon_test.go +++ b/internal/app/cmd/daemon_test.go @@ -7,6 +7,7 @@ import ( "testing" "gitea.com/gitea/runner/internal/pkg/config" + "gitea.com/gitea/runner/internal/pkg/report" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" @@ -57,10 +58,15 @@ func TestInitLoggingSetsLevelAndCaller(t *testing.T) { log.SetReportCaller(oldReportCaller) }) + oldFormatter := log.StandardLogger().Formatter + t.Cleanup(func() { log.SetFormatter(oldFormatter) }) + cfg := &config.Config{} cfg.Log.Level = "debug" initLogging(cfg) require.Equal(t, log.DebugLevel, log.GetLevel()) require.True(t, log.StandardLogger().ReportCaller) + // act plans a job on this logger, so a live task's secrets have to be masked out of it + require.IsType(t, report.MaskingFormatter(nil), log.StandardLogger().Formatter) } diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index f0fdd0b0..f25bb839 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -519,6 +519,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. Env: envs, ProxyEnv: proxyEnv, Secrets: task.Secrets, + ExtraMasks: append(proxyPasswords(), preset.Token), GitHubInstance: strings.TrimSuffix(r.client.Address(), "/"), NoSkipCheckout: true, DisableActEnv: r.cfg.Runner.SetActEnv != nil && !*r.cfg.Runner.SetActEnv, diff --git a/internal/pkg/report/globalmask.go b/internal/pkg/report/globalmask.go new file mode 100644 index 00000000..979bcbc2 --- /dev/null +++ b/internal/pkg/report/globalmask.go @@ -0,0 +1,67 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package report + +import ( + "strings" + "sync" + "sync/atomic" + + "gitea.com/gitea/runner/act/runner" + + log "github.com/sirupsen/logrus" +) + +// A job's plan runs before its job logger exists and logs through the process-wide one, which +// several tasks share, so that one holds the union of every live task's starting secrets. +var ( + globalMu sync.Mutex + globalMasks = map[*Reporter][]string{} + globalReplacer atomic.Pointer[strings.Replacer] // nil while nothing is registered +) + +func registerGlobalMasks(r *Reporter) { + globalMu.Lock() + defer globalMu.Unlock() + globalMasks[r] = r.oldnew + rebuildGlobalReplacer() +} + +func deregisterGlobalMasks(r *Reporter) { + globalMu.Lock() + defer globalMu.Unlock() + delete(globalMasks, r) + rebuildGlobalReplacer() +} + +func rebuildGlobalReplacer() { // caller holds globalMu + var oldnew []string + for _, masks := range globalMasks { + oldnew = append(oldnew, masks...) + } + if len(oldnew) == 0 { + globalReplacer.Store(nil) + return + } + globalReplacer.Store(runner.NewSecretReplacer(oldnew)) +} + +// MaskingFormatter wraps f so a registered value cannot reach the process-wide log, fields included. +func MaskingFormatter(f log.Formatter) log.Formatter { + return &maskingFormatter{inner: f} +} + +type maskingFormatter struct{ inner log.Formatter } + +func (m *maskingFormatter) Format(entry *log.Entry) ([]byte, error) { + line, err := m.inner.Format(entry) + if err != nil { + return nil, err + } + replacer := globalReplacer.Load() + if replacer == nil { // nothing to hide, so an idle daemon pays no copy + return line, nil + } + return []byte(replacer.Replace(string(line))), nil +} diff --git a/internal/pkg/report/globalmask_test.go b/internal/pkg/report/globalmask_test.go new file mode 100644 index 00000000..4720dd2c --- /dev/null +++ b/internal/pkg/report/globalmask_test.go @@ -0,0 +1,51 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package report + +import ( + "testing" + + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGlobalMasks(t *testing.T) { + formatter := MaskingFormatter(&log.TextFormatter{DisableTimestamp: true}) + format := func(job string) string { + line, err := formatter.Format(log.WithField("job", job)) + require.NoError(t, err) + return string(line) + } + register := func(oldnew ...string) *Reporter { + r := &Reporter{oldnew: oldnew} + registerGlobalMasks(r) + t.Cleanup(func() { deregisterGlobalMasks(r) }) + return r + } + + assert.Contains(t, format("build s3cr3t"), "s3cr3t") + + first := register("s3cr3t", "***") + second := register("other", "***", "otherlonger", "***") + + line := format("build s3cr3t and other") + assert.NotContains(t, line, "s3cr3t") // masked though it rode a field, not the message + assert.NotContains(t, line, "other") + assert.NotContains(t, format("otherlonger"), "longer") // longest first, so not "***longer" + + deregisterGlobalMasks(first) + line = format("build s3cr3t and other") + assert.Contains(t, line, "s3cr3t") + assert.NotContains(t, line, "other") // the task still running keeps its own + + deregisterGlobalMasks(second) + assert.Nil(t, globalReplacer.Load()) + + // A workflow could otherwise mask "error" here and rewrite every other task's log. + third := register("s3cr3t", "***") + third.addMask("runtime-secret") + assert.Contains(t, format("saw runtime-secret"), "runtime-secret") + assert.NotContains(t, third.mask("saw runtime-secret"), "runtime-secret") +} diff --git a/internal/pkg/report/reporter.go b/internal/pkg/report/reporter.go index e4733d72..5891c7eb 100644 --- a/internal/pkg/report/reporter.go +++ b/internal/pkg/report/reporter.go @@ -111,16 +111,17 @@ func NewReporter(ctx context.Context, cancel context.CancelFunc, client client.C if v := task.Context.Fields["gitea_runtime_token"].GetStringValue(); v != "" { oldnew = runner.AppendSecretMasker(oldnew, v) } - for _, v := range task.Secrets { + if v := task.Context.Fields["actions_id_token_request_token"].GetStringValue(); v != "" { oldnew = runner.AppendSecretMasker(oldnew, v) } + oldnew = runner.AppendSecretMaskers(oldnew, task.Secrets) rv := &Reporter{ ctx: ctx, cancel: cancel, client: client, oldnew: oldnew, - logReplacer: strings.NewReplacer(oldnew...), + logReplacer: runner.NewSecretReplacer(oldnew), logReportInterval: cfg.Runner.LogReportInterval, logReportMaxLatency: cfg.Runner.LogReportMaxLatency, logBatchSize: cfg.Runner.LogReportBatchSize, @@ -139,6 +140,8 @@ func NewReporter(ctx context.Context, cancel context.CancelFunc, client client.C rv.daemonWait = 6 * rv.effectiveCloseTimeout() + registerGlobalMasks(rv) + if task.Secrets["ACTIONS_STEP_DEBUG"] == "true" { rv.debugOutputEnabled = true } @@ -167,12 +170,13 @@ func (r *Reporter) Levels() []log.Level { return log.AllLevels } -// appendLogRow buffers a row for the uploader and mirrors it into job.log. A nil row is one -// the command handler dropped, such as ::add-mask::. Caller holds stateMu. +// appendLogRow masks a row before buffering it for Gitea and the local job.log, the one point +// feeding both. A nil row is one the command handler dropped. Caller holds stateMu. func (r *Reporter) appendLogRow(row *runnerv1.LogRow) { if row == nil { return } + row.Content = r.mask(row.Content) r.logRows = append(r.logRows, row) r.jobLog.write(row.Time.AsTime(), row.Content) } @@ -223,7 +227,7 @@ func (r *Reporter) Fire(entry *log.Entry) error { r.stateChanged = true if log.IsLevelEnabled(log.TraceLevel) { - log.WithFields(entry.Data).Trace(entry.Message) + log.WithFields(entry.Data).Trace(r.mask(entry.Message)) // the process masker has no ::add-mask:: value } timestamp := entry.Time @@ -445,7 +449,7 @@ func (r *Reporter) logf(format string, a ...any) { if !r.duringSteps() { // Masked like any other row: these bypass parseLogRow, but a caller can still // interpolate a secret, such as a configured URL carrying credentials. - r.appendLogRow(r.newLogRow(timestamppb.Now(), fmt.Sprintf(format, a...))) + r.appendLogRow(&runnerv1.LogRow{Time: timestamppb.Now(), Content: fmt.Sprintf(format, a...)}) } } @@ -469,6 +473,11 @@ func (r *Reporter) SetOutputs(outputs map[string]string) { r.logf("ignore output %q because the value is too long: %d > %d", k, l, maxOutputValueLen) continue } + if r.logReplacer.Replace(v) != v { // GitHub skips an output that may carry a secret rather than masking it + log.Warnf("ignore output %q because it may contain a secret", k) + r.logf("ignore output %q because it may contain a secret", k) + continue + } if _, ok := r.outputs[k]; !ok { r.outputs[k] = jobOutput{value: v} } @@ -476,6 +485,7 @@ func (r *Reporter) SetOutputs(outputs map[string]string) { } func (r *Reporter) Close(lastWords string) error { + defer deregisterGlobalMasks(r) // deferred so a panic below cannot strand this task's masks r.stateMu.Lock() r.closed = true if r.state.Result == runnerv1.Result_RESULT_UNSPECIFIED { @@ -880,22 +890,18 @@ func (r *Reporter) parseLogRow(entry *log.Entry) *runnerv1.LogRow { } } - return r.newLogRow(timestamppb.New(entry.Time), content) -} - -// newLogRow applies the masking and validation every row must carry, whatever built it. -func (r *Reporter) newLogRow(t *timestamppb.Timestamp, content string) *runnerv1.LogRow { - return &runnerv1.LogRow{ - Time: t, - Content: r.mask(content), - } + return &runnerv1.LogRow{Time: timestamppb.New(entry.Time), Content: content} } +// mask repairs the content first, so a secret the repair itself spells out is still caught. func (r *Reporter) mask(content string) string { - return strings.ToValidUTF8(r.logReplacer.Replace(content), "?") + return r.logReplacer.Replace(strings.ToValidUTF8(content, "?")) } +// addMask deliberately leaves the process-wide masker alone. Its entries come from every live +// task at once, so a workflow could otherwise mask "error" there and rewrite the runner's own +// log, and every other task's, for the rest of the job. func (r *Reporter) addMask(msg string) { r.oldnew = runner.AppendSecretMasker(r.oldnew, msg) - r.logReplacer = strings.NewReplacer(r.oldnew...) + r.logReplacer = runner.NewSecretReplacer(r.oldnew) } diff --git a/internal/pkg/report/reporter_test.go b/internal/pkg/report/reporter_test.go index 43aec80c..ebd29a34 100644 --- a/internal/pkg/report/reporter_test.go +++ b/internal/pkg/report/reporter_test.go @@ -209,7 +209,7 @@ func TestReporter_parseLogRow(t *testing.T) { got := "" if rv != nil { - got = rv.Content + got = r.mask(rv.Content) } assert.Equal(t, tt.want[idx], got) @@ -226,8 +226,7 @@ func TestReporter_parseLogRowAddMask(t *testing.T) { assert.Nil(t, r.parseLogRow(&log.Entry{Message: line}), line) - row := r.parseLogRow(&log.Entry{Message: "using supersecret now"}) - assert.Equal(t, "using *** now", row.Content, line) + assert.Equal(t, "using *** now", r.mask("using supersecret now"), line) } } @@ -1042,12 +1041,16 @@ func TestReporter_StopHeartbeats(t *testing.T) { } func TestAppendLogRow(t *testing.T) { - r := &Reporter{} - row := &runnerv1.LogRow{Time: timestamppb.Now(), Content: "hello"} + r := &Reporter{logReplacer: strings.NewReplacer("supersecret", "***")} r.appendLogRow(nil) - r.appendLogRow(row) - r.appendLogRow(nil) - assert.Equal(t, []*runnerv1.LogRow{row}, r.logRows) + r.appendLogRow(&runnerv1.LogRow{Time: timestamppb.Now(), Content: "hello supersecret"}) + require.Len(t, r.logRows, 1) + assert.Equal(t, "hello ***", r.logRows[0].Content) + + // repairing the invalid byte spells out the secret, so the repair has to come first + r = &Reporter{logReplacer: strings.NewReplacer("a?b", "***")} + r.appendLogRow(&runnerv1.LogRow{Time: timestamppb.Now(), Content: "a\xffb"}) + assert.Equal(t, "***", r.logRows[0].Content) } func TestReporter_Levels(t *testing.T) { @@ -1060,7 +1063,7 @@ func TestReporter_Result(t *testing.T) { } func TestReporter_SetOutputs(t *testing.T) { - r := &Reporter{state: &runnerv1.TaskState{}, logReplacer: strings.NewReplacer()} + r := &Reporter{state: &runnerv1.TaskState{}, logReplacer: strings.NewReplacer("s3cr3t", "***")} r.SetOutputs(map[string]string{"foo": "bar"}) got, ok := r.outputs["foo"] @@ -1083,6 +1086,17 @@ func TestReporter_SetOutputs(t *testing.T) { _, ok = r.outputs["big"] assert.False(t, ok) + // a value carrying a secret is skipped, as GitHub does, rather than sent masked + r.SetOutputs(map[string]string{"leaky": "has s3cr3t in it"}) + _, ok = r.outputs["leaky"] + assert.False(t, ok) + + // invalid UTF-8 is not a secret, so the value is kept as it is rather than dropped + r.SetOutputs(map[string]string{"binary": "caf\xff"}) + got, ok = r.outputs["binary"] + require.True(t, ok) + assert.Equal(t, "caf\xff", got.value) + // a value at exactly the limit is still stored maxValue := strings.Repeat("v", maxOutputValueLen) r.SetOutputs(map[string]string{"atlimit": maxValue}) @@ -1091,6 +1105,26 @@ func TestReporter_SetOutputs(t *testing.T) { assert.Len(t, got.value, maxOutputValueLen) } +// Gitea delivers ACTIONS_STEP_DEBUG as a secret, so masking "true" would drop any job output +// saying it. GitHub skips the same two keys. +func TestReporter_DebugSettingsAreNotMasked(t *testing.T) { + taskCtx, err := structpb.NewStruct(map[string]any{}) + require.NoError(t, err) + + reporter := NewReporter(context.Background(), nil, nil, &runnerv1.Task{ + Context: taskCtx, + Secrets: map[string]string{"ACTIONS_STEP_DEBUG": "true", "ACTIONS_RUNNER_DEBUG": "true", "TOKEN": "s3cr3t"}, + }, &config.Config{}) + defer deregisterGlobalMasks(reporter) + + assert.True(t, reporter.debugOutputEnabled) + assert.Equal(t, "debug is true", reporter.mask("debug is true")) + assert.Equal(t, "***", reporter.mask("s3cr3t")) + + reporter.SetOutputs(map[string]string{"changed": "true"}) + assert.Equal(t, "true", reporter.outputs["changed"].value) // needs..outputs.changed == 'true' still works +} + // An output the server acknowledged is not reported again. func TestReporter_OutputsSentOnce(t *testing.T) { client := mocks.NewClient(t) @@ -1160,11 +1194,10 @@ func TestReporter_masksEncodedSecrets(t *testing.T) { "basic " + base64.StdEncoding.EncodeToString([]byte(secret)), "https://example.com/?token=" + url.QueryEscape(secret), } { - row := r.parseLogRow(&log.Entry{Message: line}) - require.NotNil(t, row) - assert.Contains(t, row.Content, "***") - assert.NotContains(t, row.Content, secret) - assert.NotContains(t, row.Content, base64.StdEncoding.EncodeToString([]byte(secret))) + masked := r.mask(line) + assert.Contains(t, masked, "***") + assert.NotContains(t, masked, secret) + assert.NotContains(t, masked, base64.StdEncoding.EncodeToString([]byte(secret))) } } @@ -1297,6 +1330,45 @@ func TestReporter_NoteReport(t *testing.T) { assert.Contains(t, hook.LastEntry().Message, "reconnected") } +// A job's final error can carry a secret, e.g. one interpolated into a failing +// expression, so Close must mask it like every other row. +func TestReporter_CloseMasksLastWords(t *testing.T) { + const secret = "supersecret" + var rows []*runnerv1.LogRow + + client := mocks.NewClient(t) + client.On("UpdateLog", mock.Anything, mock.Anything).Return( + func(_ context.Context, req *connect_go.Request[runnerv1.UpdateLogRequest]) (*connect_go.Response[runnerv1.UpdateLogResponse], error) { + rows = append(rows, req.Msg.Rows...) + return connect_go.NewResponse(&runnerv1.UpdateLogResponse{ + AckIndex: req.Msg.Index + int64(len(req.Msg.Rows)), + }), nil + }, + ) + client.On("UpdateTask", mock.Anything, mock.Anything).Return( + func(_ context.Context, _ *connect_go.Request[runnerv1.UpdateTaskRequest]) (*connect_go.Response[runnerv1.UpdateTaskResponse], error) { + return connect_go.NewResponse(&runnerv1.UpdateTaskResponse{}), nil + }, + ) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + const idToken = "id-token-request-secret" + taskCtx, err := structpb.NewStruct(map[string]any{"actions_id_token_request_token": idToken}) + require.NoError(t, err) + cfg, _ := config.LoadDefault("") + reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{ + Context: taskCtx, + Secrets: map[string]string{"TOKEN": secret}, + }, cfg) + close(reporter.daemon) + + require.NoError(t, reporter.Close("could not get job matrix: "+secret+" "+idToken)) + + require.Len(t, rows, 1) + assert.Equal(t, "could not get job matrix: *** ***", rows[0].Content) +} + // giteaLogModel mirrors how Gitea stores a task log: UpdateLog appends rows to one // stream, and UpdateTask overwrites the per-step ranges the web UI slices that stream by // (modules/actions/task_state.go, FullSteps).